[PATCH] Plumb through xen-platform device logging

[PATCH] Plumb through xen-platform device logging

[Paul Durrant]

The xen-platform device uses the QEMU trace framework to log but nothing was plumbing this
through to the log file. I've therefore added the appropriate rune to the QEMU configure
command line to enable the stderr backend, added an example qemu-trace-events file (that just
enables the xen_platform_log event) and added the necessary argument to the QEMU command line
that libxl generates to suck in the events file.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 tools/Makefile                   |    3 ++-
 tools/examples/Makefile          |    1 +
 tools/examples/qemu-trace-events |    2 ++
 tools/libxl/libxl_dm.c           |    7 +++++++
 tools/libxl/libxl_paths.c        |    5 +++++
 5 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 tools/examples/qemu-trace-events

diff --git a/tools/Makefile b/tools/Makefile
index e44a3e9..2d791a4 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -202,7 +202,8 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
 		--disable-kvm \
 		--disable-docs \
 		--python=$(PYTHON) \
-		$(IOEMU_CONFIGURE_CROSS); \
+		$(IOEMU_CONFIGURE_CROSS) \
+		--enable-trace-backend=stderr; \
 	$(MAKE) all
 
 subdir-install-qemu-xen-dir: subdir-all-qemu-xen-dir
diff --git a/tools/examples/Makefile b/tools/examples/Makefile
index 8cea724..70facb4 100644
--- a/tools/examples/Makefile
+++ b/tools/examples/Makefile
@@ -25,6 +25,7 @@ XEN_CONFIGS += xend-pci-quirks.sxp
 XEN_CONFIGS += xend-pci-permissive.sxp
 XEN_CONFIGS += xl.conf
 XEN_CONFIGS += cpupool
+XEN_CONFIGS += qemu-trace-events
 
 .PHONY: all
 all:
diff --git a/tools/examples/qemu-trace-events b/tools/examples/qemu-trace-events
new file mode 100644
index 0000000..8e316fe
--- /dev/null
+++ b/tools/examples/qemu-trace-events
@@ -0,0 +1,2 @@
+# Enable Xen PCI Platform device logging
+xen_platform_log
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 2924298..35f71cc 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -370,6 +370,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                       "-xen-domid",
                       libxl__sprintf(gc, "%d", guest_domid), NULL);
 
+    flexarray_vappend(dm_args,
+                      "-trace",
+                      libxl__sprintf(gc,
+                                     "events=%s/qemu-trace-events",
+                                     libxl__xen_config_dir_path()),
+                      NULL);
+
     flexarray_append(dm_args, "-chardev");
     flexarray_append(dm_args,
                      libxl__sprintf(gc, "socket,id=libxl-cmd,"
diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
index 14b28d1..12993ec 100644
--- a/tools/libxl/libxl_paths.c
+++ b/tools/libxl/libxl_paths.c
@@ -40,6 +40,11 @@ const char *libxl__run_dir_path(void)
     return XEN_RUN_DIR;
 }
 
+const char *libxl__xen_config_dir_path(void)
+{
+    return XEN_CONFIG_DIR;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
1.7.10.4

Re: [PATCH] Plumb through xen-platform device logging

[Paul Durrant]

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 04 July 2013 16:33
> To: xen-devel@lists.xen.org
> Cc: Paul Durrant
> Subject: [PATCH] Plumb through xen-platform device logging
> 
> The xen-platform device uses the QEMU trace framework to log but nothing
> was plumbing this
> through to the log file. I've therefore added the appropriate rune to the
> QEMU configure
> command line to enable the stderr backend, added an example qemu-trace-
> events file (that just
> enables the xen_platform_log event) and added the necessary argument to
> the QEMU command line
> that libxl generates to suck in the events file.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

Ping?

> ---
>  tools/Makefile                   |    3 ++-
>  tools/examples/Makefile          |    1 +
>  tools/examples/qemu-trace-events |    2 ++
>  tools/libxl/libxl_dm.c           |    7 +++++++
>  tools/libxl/libxl_paths.c        |    5 +++++
>  5 files changed, 17 insertions(+), 1 deletion(-)
>  create mode 100644 tools/examples/qemu-trace-events
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index e44a3e9..2d791a4 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -202,7 +202,8 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>  		--disable-kvm \
>  		--disable-docs \
>  		--python=$(PYTHON) \
> -		$(IOEMU_CONFIGURE_CROSS); \
> +		$(IOEMU_CONFIGURE_CROSS) \
> +		--enable-trace-backend=stderr; \
>  	$(MAKE) all
> 
>  subdir-install-qemu-xen-dir: subdir-all-qemu-xen-dir
> diff --git a/tools/examples/Makefile b/tools/examples/Makefile
> index 8cea724..70facb4 100644
> --- a/tools/examples/Makefile
> +++ b/tools/examples/Makefile
> @@ -25,6 +25,7 @@ XEN_CONFIGS += xend-pci-quirks.sxp
>  XEN_CONFIGS += xend-pci-permissive.sxp
>  XEN_CONFIGS += xl.conf
>  XEN_CONFIGS += cpupool
> +XEN_CONFIGS += qemu-trace-events
> 
>  .PHONY: all
>  all:
> diff --git a/tools/examples/qemu-trace-events b/tools/examples/qemu-
> trace-events
> new file mode 100644
> index 0000000..8e316fe
> --- /dev/null
> +++ b/tools/examples/qemu-trace-events
> @@ -0,0 +1,2 @@
> +# Enable Xen PCI Platform device logging
> +xen_platform_log
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 2924298..35f71cc 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -370,6 +370,13 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
>                        "-xen-domid",
>                        libxl__sprintf(gc, "%d", guest_domid), NULL);
> 
> +    flexarray_vappend(dm_args,
> +                      "-trace",
> +                      libxl__sprintf(gc,
> +                                     "events=%s/qemu-trace-events",
> +                                     libxl__xen_config_dir_path()),
> +                      NULL);
> +
>      flexarray_append(dm_args, "-chardev");
>      flexarray_append(dm_args,
>                       libxl__sprintf(gc, "socket,id=libxl-cmd,"
> diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
> index 14b28d1..12993ec 100644
> --- a/tools/libxl/libxl_paths.c
> +++ b/tools/libxl/libxl_paths.c
> @@ -40,6 +40,11 @@ const char *libxl__run_dir_path(void)
>      return XEN_RUN_DIR;
>  }
> 
> +const char *libxl__xen_config_dir_path(void)
> +{
> +    return XEN_CONFIG_DIR;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> --
> 1.7.10.4

Re: [PATCH] Plumb through xen-platform device logging

[Ian Campbell]

On Thu, 2013-07-11 at 08:23 +0000, Paul Durrant wrote:
> > diff --git a/tools/Makefile b/tools/Makefile
> > index e44a3e9..2d791a4 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -202,7 +202,8 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
> >  		--disable-kvm \
> >  		--disable-docs \
> >  		--python=$(PYTHON) \
> > -		$(IOEMU_CONFIGURE_CROSS); \
> > +		$(IOEMU_CONFIGURE_CROSS) \
> > +		--enable-trace-backend=stderr; \

Is this the only way to do this?

It is likely that many distros will use their standard qemu packages
which may or may not have this option enabled.

I'm not sure if this is a static always on option or if this is simply
making a trace backend available for use.

Looking at http://wiki.qemu.org/Features/Tracing is the tracing
interface really the right way to be logging this particular class of
information? I'd have thought a simple logfile support in the platform
device would be a much more natural fit.

> >  	$(MAKE) all
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 2924298..35f71cc 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -370,6 +370,13 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >                        "-xen-domid",
> >                        libxl__sprintf(gc, "%d", guest_domid), NULL);
> > 
> > +    flexarray_vappend(dm_args,
> > +                      "-trace",
> > +                      libxl__sprintf(gc,
> > +                                     "events=%s/qemu-trace-events",
> > +                                     libxl__xen_config_dir_path()),
> > +                      NULL);

Doesn't this end up logging to /etc/xen? Not what we want I think.

or maybe this is just the config file, which, apart from my comments
about the suitability of the interface above, makes me wonder where the
logs do go? Ideally they would go to /var/log/xen/qemu-blah-name.log not
to xl stdout.

Re: [PATCH] Plumb through xen-platform device logging

[Paul Durrant]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: 11 July 2013 12:44
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] Plumb through xen-platform device logging
> 
> On Thu, 2013-07-11 at 08:23 +0000, Paul Durrant wrote:
> > > diff --git a/tools/Makefile b/tools/Makefile
> > > index e44a3e9..2d791a4 100644
> > > --- a/tools/Makefile
> > > +++ b/tools/Makefile
> > > @@ -202,7 +202,8 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
> > >  		--disable-kvm \
> > >  		--disable-docs \
> > >  		--python=$(PYTHON) \
> > > -		$(IOEMU_CONFIGURE_CROSS); \
> > > +		$(IOEMU_CONFIGURE_CROSS) \
> > > +		--enable-trace-backend=stderr; \
> 
> Is this the only way to do this?
> 
> It is likely that many distros will use their standard qemu packages
> which may or may not have this option enabled.
>

That's true.
 
> I'm not sure if this is a static always on option or if this is simply
> making a trace backend available for use.
> 

It's a static selection as far as I can tell. The tracing backend is not selectable at runtime :-(

> Looking at http://wiki.qemu.org/Features/Tracing is the tracing
> interface really the right way to be logging this particular class of
> information? I'd have thought a simple logfile support in the platform
> device would be a much more natural fit.
> 

That makes sense to me, but whoever coded up the platform device obviously believed tracing to be the correct way to log. I don't know the history of that decision.
It doesn't change the fact though that, currently, xen builds of QEMU don't configure any form of tracing backend at all which doesn't seem particularly helpful, and I did introduce platform logging as an example of an event to log so I think the patch is useful as far as it goes, but maybe another patch to the platform device in QEMU would also be considered a good idea.

  Paul

> > >  	$(MAKE) all
> > >
> > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > index 2924298..35f71cc 100644
> > > --- a/tools/libxl/libxl_dm.c
> > > +++ b/tools/libxl/libxl_dm.c
> > > @@ -370,6 +370,13 @@ static char **
> > > libxl__build_device_model_args_new(libxl__gc *gc,
> > >                        "-xen-domid",
> > >                        libxl__sprintf(gc, "%d", guest_domid), NULL);
> > >
> > > +    flexarray_vappend(dm_args,
> > > +                      "-trace",
> > > +                      libxl__sprintf(gc,
> > > +                                     "events=%s/qemu-trace-events",
> > > +                                     libxl__xen_config_dir_path()),
> > > +                      NULL);
> 
> Doesn't this end up logging to /etc/xen? Not what we want I think.
> 
> or maybe this is just the config file, which, apart from my comments
> about the suitability of the interface above, makes me wonder where the
> logs do go? Ideally they would go to /var/log/xen/qemu-blah-name.log not
> to xl stdout.
> 

Re: [PATCH] Plumb through xen-platform device logging

[Don Slutz]

On 07/11/13 08:07, Paul Durrant wrote:
>> -----Original Message-----
>> From: Ian Campbell [mailto:ian.campbell@citrix.com]
>> Sent: 11 July 2013 12:44
>> To: Paul Durrant
>> Cc: xen-devel@lists.xen.org
>> Subject: Re: [Xen-devel] [PATCH] Plumb through xen-platform device logging
>>
>> On Thu, 2013-07-11 at 08:23 +0000, Paul Durrant wrote:
>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>> index e44a3e9..2d791a4 100644
>>>> --- a/tools/Makefile
>>>> +++ b/tools/Makefile
>>>> @@ -202,7 +202,8 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>>>>   		--disable-kvm \
>>>>   		--disable-docs \
>>>>   		--python=$(PYTHON) \
>>>> -		$(IOEMU_CONFIGURE_CROSS); \
>>>> +		$(IOEMU_CONFIGURE_CROSS) \
>>>> +		--enable-trace-backend=stderr; \
I have done this in a more flexable way:

--- a/tools/Makefile
+++ b/tools/Makefile

@@ -187,8 +185,7 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
                 source=.; \
         fi; \
         cd qemu-xen-dir; \
-       $$source/configure --enable-xen --target-list=i386-softmmu \
-               --prefix=$(PREFIX) \
+       $$source/configure --enable-xen $(QEMU_UPSTREAM_EXTRA_CONFIG) 
--target-list=i386-softmmu \
                 --source-path=$$source \
                 --extra-cflags="-I$(XEN_ROOT)/tools/include \
                 -I$(XEN_ROOT)/tools/libxc \

I.E by adding QEMU_UPSTREAM_EXTRA_CONFIG which I add to .config when I 
want to turn on tracing.

>> Is this the only way to do this?
>>
>> It is likely that many distros will use their standard qemu packages
>> which may or may not have this option enabled.
>>
> That's true.
>   
>> I'm not sure if this is a static always on option or if this is simply
>> making a trace backend available for use.
>>
> It's a static selection as far as I can tell. The tracing backend is not selectable at runtime :-(
It is static and adds a lot of code.  So you want a way to not configure 
it also.  However what trace output happens is changeable while running.
>> Looking at http://wiki.qemu.org/Features/Tracing is the tracing
>> interface really the right way to be logging this particular class of
>> information? I'd have thought a simple logfile support in the platform
>> device would be a much more natural fit.
>>
> That makes sense to me, but whoever coded up the platform device obviously believed tracing to be the correct way to log. I don't know the history of that decision.
> It doesn't change the fact though that, currently, xen builds of QEMU don't configure any form of tracing backend at all which doesn't seem particularly helpful, and I did introduce platform logging as an example of an event to log so I think the patch is useful as far as it goes, but maybe another patch to the platform device in QEMU would also be considered a good idea.
>
>    Paul
>
>>>>   	$(MAKE) all
>>>>
>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>> index 2924298..35f71cc 100644
>>>> --- a/tools/libxl/libxl_dm.c
>>>> +++ b/tools/libxl/libxl_dm.c
>>>> @@ -370,6 +370,13 @@ static char **
>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>                         "-xen-domid",
>>>>                         libxl__sprintf(gc, "%d", guest_domid), NULL);
>>>>
>>>> +    flexarray_vappend(dm_args,
>>>> +                      "-trace",
>>>> +                      libxl__sprintf(gc,
>>>> +                                     "events=%s/qemu-trace-events",
>>>> +                                     libxl__xen_config_dir_path()),
>>>> +                      NULL);
>> Doesn't this end up logging to /etc/xen? Not what we want I think.
>>
>> or maybe this is just the config file, which, apart from my comments
>> about the suitability of the interface above, makes me wonder where the
>> logs do go? Ideally they would go to /var/log/xen/qemu-blah-name.log not
>> to xl stdout.
>>
Using --enable-trace-backend=stderr, the output ends up in 
/var/log/xen/qemu-dm-P-1-0.log

I.E. where all stderr from qemu does.
    -Don Slutz
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] Plumb through xen-platform device logging

[Don Slutz]

On 07/11/13 08:07, Paul Durrant wrote:
>> -----Original Message-----
>> From: Ian Campbell [mailto:ian.campbell@citrix.com]
>> Sent: 11 July 2013 12:44
>> To: Paul Durrant
>> Cc: xen-devel@lists.xen.org
>> Subject: Re: [Xen-devel] [PATCH] Plumb through xen-platform device logging
>>
>> On Thu, 2013-07-11 at 08:23 +0000, Paul Durrant wrote:
>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>> index e44a3e9..2d791a4 100644
>>>> --- a/tools/Makefile
>>>> +++ b/tools/Makefile
>>>> @@ -202,7 +202,8 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>>>>   		--disable-kvm \
>>>>   		--disable-docs \
>>>>   		--python=$(PYTHON) \
>>>> -		$(IOEMU_CONFIGURE_CROSS); \
>>>> +		$(IOEMU_CONFIGURE_CROSS) \
>>>> +		--enable-trace-backend=stderr; \
I have done this in a more flexable way:

--- a/tools/Makefile
+++ b/tools/Makefile

@@ -187,8 +185,7 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
                 source=.; \
         fi; \
         cd qemu-xen-dir; \
-       $$source/configure --enable-xen --target-list=i386-softmmu \
-               --prefix=$(PREFIX) \
+       $$source/configure --enable-xen $(QEMU_UPSTREAM_EXTRA_CONFIG) 
--target-list=i386-softmmu \
                 --source-path=$$source \
                 --extra-cflags="-I$(XEN_ROOT)/tools/include \
                 -I$(XEN_ROOT)/tools/libxc \

I.E by adding QEMU_UPSTREAM_EXTRA_CONFIG which I add to .config when I 
want to turn on tracing.

>> Is this the only way to do this?
>>
>> It is likely that many distros will use their standard qemu packages
>> which may or may not have this option enabled.
>>
> That's true.
>   
>> I'm not sure if this is a static always on option or if this is simply
>> making a trace backend available for use.
>>
> It's a static selection as far as I can tell. The tracing backend is not selectable at runtime :-(
It is static and adds a lot of code.  So you want a way to not configure 
it also.  However what trace output happens is changeable while running.
>> Looking at http://wiki.qemu.org/Features/Tracing is the tracing
>> interface really the right way to be logging this particular class of
>> information? I'd have thought a simple logfile support in the platform
>> device would be a much more natural fit.
>>
> That makes sense to me, but whoever coded up the platform device obviously believed tracing to be the correct way to log. I don't know the history of that decision.
> It doesn't change the fact though that, currently, xen builds of QEMU don't configure any form of tracing backend at all which doesn't seem particularly helpful, and I did introduce platform logging as an example of an event to log so I think the patch is useful as far as it goes, but maybe another patch to the platform device in QEMU would also be considered a good idea.
>
>    Paul
>
>>>>   	$(MAKE) all
>>>>
>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>> index 2924298..35f71cc 100644
>>>> --- a/tools/libxl/libxl_dm.c
>>>> +++ b/tools/libxl/libxl_dm.c
>>>> @@ -370,6 +370,13 @@ static char **
>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>                         "-xen-domid",
>>>>                         libxl__sprintf(gc, "%d", guest_domid), NULL);
>>>>
>>>> +    flexarray_vappend(dm_args,
>>>> +                      "-trace",
>>>> +                      libxl__sprintf(gc,
>>>> +                                     "events=%s/qemu-trace-events",
>>>> +                                     libxl__xen_config_dir_path()),
>>>> +                      NULL);
I handle this by adjusting device_model_args_hvm like:

device_model_args_hvm = [ "-trace", "events=/etc/xen/qemu-trace-events" ]

in the config.
>> Doesn't this end up logging to /etc/xen? Not what we want I think.
>>
>> or maybe this is just the config file, which, apart from my comments
>> about the suitability of the interface above, makes me wonder where the
>> logs do go? Ideally they would go to /var/log/xen/qemu-blah-name.log not
>> to xl stdout.
>>
Using --enable-trace-backend=stderr, the output ends up in 
/var/log/xen/qemu-dm-P-1-0.log

I.E. where all stderr from qemu does.
    -Don Slutz
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] Plumb through xen-platform device logging

[Paul Durrant]

> -----Original Message-----
> From: Don Slutz [mailto:dslutz@verizon.com]
> Sent: 11 July 2013 13:29
> To: Paul Durrant
> Cc: Ian Campbell; xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] Plumb through xen-platform device logging
> 
> On 07/11/13 08:07, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> >> Sent: 11 July 2013 12:44
> >> To: Paul Durrant
> >> Cc: xen-devel@lists.xen.org
> >> Subject: Re: [Xen-devel] [PATCH] Plumb through xen-platform device
> logging
> >>
> >> On Thu, 2013-07-11 at 08:23 +0000, Paul Durrant wrote:
> >>>> diff --git a/tools/Makefile b/tools/Makefile
> >>>> index e44a3e9..2d791a4 100644
> >>>> --- a/tools/Makefile
> >>>> +++ b/tools/Makefile
> >>>> @@ -202,7 +202,8 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
> >>>>   		--disable-kvm \
> >>>>   		--disable-docs \
> >>>>   		--python=$(PYTHON) \
> >>>> -		$(IOEMU_CONFIGURE_CROSS); \
> >>>> +		$(IOEMU_CONFIGURE_CROSS) \
> >>>> +		--enable-trace-backend=stderr; \
> I have done this in a more flexable way:
> 
> --- a/tools/Makefile
> +++ b/tools/Makefile
> 
> @@ -187,8 +185,7 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>                  source=.; \
>          fi; \
>          cd qemu-xen-dir; \
> -       $$source/configure --enable-xen --target-list=i386-softmmu \
> -               --prefix=$(PREFIX) \
> +       $$source/configure --enable-xen
> $(QEMU_UPSTREAM_EXTRA_CONFIG)
> --target-list=i386-softmmu \
>                  --source-path=$$source \
>                  --extra-cflags="-I$(XEN_ROOT)/tools/include \
>                  -I$(XEN_ROOT)/tools/libxc \
> 
> I.E by adding QEMU_UPSTREAM_EXTRA_CONFIG which I add to .config
> when I
> want to turn on tracing.
> 

Perhaps we should have that, and maybe a dedicated QEMU_TRACE_BACKEND that defaults to stderr?

  Paul

Re: [PATCH] Plumb through xen-platform device logging

[Ian Campbell]

On Thu, 2013-07-11 at 13:07 +0100, Paul Durrant wrote:

> > Looking at http://wiki.qemu.org/Features/Tracing is the tracing
> > interface really the right way to be logging this particular class of
> > information? I'd have thought a simple logfile support in the platform
> > device would be a much more natural fit.
> > 
> 
> That makes sense to me, but whoever coded up the platform device
> obviously believed tracing to be the correct way to log. I don't know
> the history of that decision.

I guess either Anthony or Stefano knows. Do you guys know why we log the
platform device I/O port debug via the trace subsystems? It doesn't seem
like a good fit.

A better fit would be the qemu chr subsystem (I think that's the name, I
mean the thing which lets you direct serial/parallel etc to
file,tcp,sockets etc etc.)

> It doesn't change the fact though that, currently, xen builds of QEMU
> don't configure any form of tracing backend at all which doesn't seem
> particularly helpful, and I did introduce platform logging as an
> example of an event to log so I think the patch is useful as far as it
> goes, but maybe another patch to the platform device in QEMU would
> also be considered a good idea.
> 
>   Paul
> 
> > > >  	$(MAKE) all
> > > >
> > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > index 2924298..35f71cc 100644
> > > > --- a/tools/libxl/libxl_dm.c
> > > > +++ b/tools/libxl/libxl_dm.c
> > > > @@ -370,6 +370,13 @@ static char **
> > > > libxl__build_device_model_args_new(libxl__gc *gc,
> > > >                        "-xen-domid",
> > > >                        libxl__sprintf(gc, "%d", guest_domid), NULL);
> > > >
> > > > +    flexarray_vappend(dm_args,
> > > > +                      "-trace",
> > > > +                      libxl__sprintf(gc,
> > > > +                                     "events=%s/qemu-trace-events",
> > > > +                                     libxl__xen_config_dir_path()),
> > > > +                      NULL);
> > 
> > Doesn't this end up logging to /etc/xen? Not what we want I think.
> > 
> > or maybe this is just the config file, which, apart from my comments
> > about the suitability of the interface above, makes me wonder where the
> > logs do go? Ideally they would go to /var/log/xen/qemu-blah-name.log not
> > to xl stdout.
> > 
> 

Re: [PATCH] Plumb through xen-platform device logging

[Anthony PERARD]

On 11/07/13 13:50, Ian Campbell wrote:
> On Thu, 2013-07-11 at 13:07 +0100, Paul Durrant wrote:
> 
>>> > > Looking at http://wiki.qemu.org/Features/Tracing is the tracing
>>> > > interface really the right way to be logging this particular class of
>>> > > information? I'd have thought a simple logfile support in the platform
>>> > > device would be a much more natural fit.
>>> > > 
>> > 
>> > That makes sense to me, but whoever coded up the platform device
>> > obviously believed tracing to be the correct way to log. I don't know
>> > the history of that decision.
> I guess either Anthony or Stefano knows. Do you guys know why we log the
> platform device I/O port debug via the trace subsystems? It doesn't seem
> like a good fit.

It seams that I made this choice long time ago.
http://marc.info/?l=xen-devel&m=129864357001108&w=2

But I never try to use it, that was maybe not a great choice. After a
quick look into the qemu tree, using the trace thing those not seams bad
as well. There is just few step:

 - compile qemu with traces enable
 - adding to the vm-xl-config this:
device_model_args_hvm = [ '-trace', 'events=/tmp/traces' ]
(with "xen_platform_log" in /tmp/traces)


So, about the patch, I don't feel a good idea to have it enable all the
time for all the guest. Also, QEMU will refuse to start if it's compiled
without trace support.

> A better fit would be the qemu chr subsystem (I think that's the name, I
> mean the thing which lets you direct serial/parallel etc to
> file,tcp,sockets etc etc.)

This can maybe be done using a property, which could be set via the
command line by using the -device options.

-- 
Anthony PERARD

Re: [PATCH] Plumb through xen-platform device logging

[Paul Durrant]

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 11 July 2013 14:54
> To: Ian Campbell
> Cc: Paul Durrant; xen-devel@lists.xen.org; Stefano Stabellini
> Subject: Re: [Xen-devel] [PATCH] Plumb through xen-platform device logging
> 
> On 11/07/13 13:50, Ian Campbell wrote:
> > On Thu, 2013-07-11 at 13:07 +0100, Paul Durrant wrote:
> >
> >>> > > Looking at http://wiki.qemu.org/Features/Tracing is the tracing
> >>> > > interface really the right way to be logging this particular class of
> >>> > > information? I'd have thought a simple logfile support in the platform
> >>> > > device would be a much more natural fit.
> >>> > >
> >> >
> >> > That makes sense to me, but whoever coded up the platform device
> >> > obviously believed tracing to be the correct way to log. I don't know
> >> > the history of that decision.
> > I guess either Anthony or Stefano knows. Do you guys know why we log
> the
> > platform device I/O port debug via the trace subsystems? It doesn't seem
> > like a good fit.
> 
> It seams that I made this choice long time ago.
> http://marc.info/?l=xen-devel&m=129864357001108&w=2
> 
> But I never try to use it, that was maybe not a great choice. After a
> quick look into the qemu tree, using the trace thing those not seams bad
> as well. There is just few step:
> 
>  - compile qemu with traces enable
>  - adding to the vm-xl-config this:
> device_model_args_hvm = [ '-trace', 'events=/tmp/traces' ]
> (with "xen_platform_log" in /tmp/traces)
>

Yeah, that's what I had before :-)

 > 
> So, about the patch, I don't feel a good idea to have it enable all the
> time for all the guest. Also, QEMU will refuse to start if it's compiled
> without trace support.
> 

That’s a good point. Should probably just have a minimal patch to the xen build to enable a tracing backend (under config control) and leave the libxl stuff out.

  Paul

> > A better fit would be the qemu chr subsystem (I think that's the name, I
> > mean the thing which lets you direct serial/parallel etc to
> > file,tcp,sockets etc etc.)
> 
> This can maybe be done using a property, which could be set via the
> command line by using the -device options.
> 
> --
> Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] Plumb through xen-platform device logging

[Ian Campbell]

On Thu, 2013-07-11 at 13:07 +0100, Paul Durrant wrote:
> It doesn't change the fact though that, currently, xen builds of QEMU
> don't configure any form of tracing backend at all which doesn't seem
> particularly helpful, and I did introduce platform logging as an
> example of an event to log so I think the patch is useful as far as it
> goes, but maybe another patch to the platform device in QEMU would
> also be considered a good idea.

I think if we do anything we should do it properly (i.e. fix qemu to do
sensible logging) rather than layering more stuff on top of the wrong
thing.

Ian.

Re: [PATCH] Plumb through xen-platform device logging

[Paul Durrant]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: 11 July 2013 15:02
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] Plumb through xen-platform device logging
> 
> On Thu, 2013-07-11 at 13:07 +0100, Paul Durrant wrote:
> > It doesn't change the fact though that, currently, xen builds of QEMU
> > don't configure any form of tracing backend at all which doesn't seem
> > particularly helpful, and I did introduce platform logging as an
> > example of an event to log so I think the patch is useful as far as it
> > goes, but maybe another patch to the platform device in QEMU would
> > also be considered a good idea.
> 
> I think if we do anything we should do it properly (i.e. fix qemu to do
> sensible logging) rather than layering more stuff on top of the wrong
> thing.
> 

Sure. I'll get together a patch to allow QEMU tracing to be configured in a xen build anyway, but then I'll do a QEMU patch to take a logfile parameter into the platform device model.

  Paul