Re: [Xen-devel] [PATCH v2 3/7] xen/vm-events: Move monitor_domctl to common-side.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.02.16 at 16:52, <czuzu@xxxxxxxxxxxxxxx> wrote:
>  xen/arch/x86/Kconfig                              |   4 +
>  xen/arch/x86/Makefile                             |   2 +-
>  xen/arch/x86/hvm/event.c                          |   2 +-
>  xen/arch/x86/hvm/hvm.c                            |   2 +-
>  xen/arch/x86/hvm/vmx/vmx.c                        |   2 +-
>  xen/arch/x86/monitor_x86.c                        |  72 ++++++++
>  xen/common/Kconfig                                |  20 +++
>  xen/common/Makefile                               |   1 +
>  xen/common/domctl.c                               |   2 +-
>  xen/{arch/x86 => common}/monitor.c                | 195 
> +++++++++-------------
>  xen/include/asm-arm/{monitor.h => monitor_arch.h} |  34 +++-
>  xen/include/asm-x86/monitor_arch.h                |  74 ++++++++
>  xen/include/{asm-x86 => xen}/monitor.h            |  17 +-
>  13 files changed, 293 insertions(+), 134 deletions(-)
>  create mode 100644 xen/arch/x86/monitor_x86.c
>  rename xen/{arch/x86 => common}/monitor.c (44%)
>  rename xen/include/asm-arm/{monitor.h => monitor_arch.h} (46%)
>  create mode 100644 xen/include/asm-x86/monitor_arch.h
>  rename xen/include/{asm-x86 => xen}/monitor.h (74%)

With percentages as low as 44 I'm not sure all this strange
renaming and introduction of oddly named new files is actually a
good idea.

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -14,6 +14,10 @@ config X86
>       select HAS_MEM_ACCESS
>       select HAS_MEM_PAGING
>       select HAS_MEM_SHARING
> +     select HAS_VM_EVENT_WRITE_CTRLREG
> +     select HAS_VM_EVENT_SINGLESTEP
> +     select HAS_VM_EVENT_SOFTWARE_BREAKPOINT
> +     select HAS_VM_EVENT_GUEST_REQUEST
>       select HAS_NS16550
>       select HAS_PASSTHROUGH
>       select HAS_PCI

Please don't break the alphabetic ordering.

> --- a/xen/arch/x86/monitor.c
> +++ b/xen/common/monitor.c
> @@ -1,9 +1,10 @@
>  /*
> - * arch/x86/monitor.c
> + * xen/common/monitor.c
>   *
> - * Architecture-specific monitor_op domctl handler.
> + * Common monitor_op domctl handler.
>   *
>   * Copyright (c) 2015 Tamas K Lengyel (tamas@xxxxxxxxxxxxx)
> + * Copyright (c) 2016, Bitdefender S.R.L.
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public
> @@ -18,101 +19,66 @@
>   * License along with this program; If not, see 
> <http://www.gnu.org/licenses/>.
>   */
>  
> -#include <xen/config.h>
> -#include <xen/sched.h>
> -#include <xen/mm.h>
> -#include <asm/domain.h>
> -#include <asm/monitor.h>
> -#include <public/domctl.h>
> +#include <xen/monitor.h>
> +#include <xen/sched.h>              /* for domain_pause, ... */
> +#include <xen/config.h>             /* for XENLOG_WARNING */

Please simply drop this include, it's being automatically included via
compiler command line option. Also please avoid comments like this
unless they explain an otherwise unexpected or unreasonable
inclusion.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel