Xen devel - Dec 2012 - Upstream qemu-xen, log verbosity and compile errors when enabling debug, filenaming

Hi All,

Yesterday I have tried building and using upstream qemu and seabios.
Config.mk:
QEMU_UPSTREAM_URL ?= git://git.qemu.org/qemu.git
QEMU_UPSTREAM_REVISION ?= master

SEABIOS_UPSTREAM_URL ?= git://git.qemu.org/seabios.git
SEABIOS_UPSTREAM_TAG ?= master

And i''m happy to say that it works quite ok, even with (secondary pci)
pci-passthrough ( using an ATI gfx adapter and windows7 as guest os) :-).

But it seems to have an issue with a USB controller which is trying to use msi-X
interrupts, which makes xl dmesg report:
(XEN) [2012-12-06 16:07:24] vmsi.c:108:d32767 Unsupported delivery mode 3
and when "pci_msitranslate=0" is set the error still occurs, only this
time the correct domain number is reported, instead of the 32767.


However, when trying to debug, i noticed although making a debug build (make
debug=y && make debug=y install), qemu-dm-<guestname>.log stays
almost empty.
It seems all the defines related to debugging are not set.

- Would it be appropriated to enable them all when making a debug build ?
- Would it be wise to also have some more verbose logging when not running a
debug build ?
- And if yes, what would be the nicest way to set the defines ?
- Should the naming of the debug defines be made more consistent ?


When enabling these debug defines by hand:

xen-all.c
#define DEBUG_XEN

xen-mapcache.c
#define MAPCACHE_DEBUG

hw/xen-host-pci-device.c
#define XEN_HOST_PCI_DEVICE_DEBUG

hw/xen_platform.c
#define DEBUG_PLATFORM

hw/xen_pt.h
#define XEN_PT_LOGGING_ENABLED
#define XEN_PT_DEBUG_PCI_CONFIG_ACCESS

I get a lot of compile errors related to wrong types in the debug
printf''s.


Another thing that occurred to me was that the file naming doesn''t seem
to be overly consistent:

xen-all.c
xen-mapcache.c
xen-mapcache.h
xen-stub.c
xen_apic.c
hw/xen_backend.c
hw/xen_backend.h
hw/xen_blkif.h
hw/xen_common.h
hw/xen_console.c
hw/xen_devconfig.c
hw/xen_disk.c
hw/xen_domainbuild.c
hw/xen_domainbuild.h
hw/xenfb.c
hw/xen.h
hw/xen-host-pci-device.c
hw/xen-host-pci-device.h
hw/xen_machine_pv.c
hw/xen_nic.c
hw/xen_platform.c
hw/xen_pt.c
hw/xen_pt_config_init.c
hw/xen_pt.h
hw/xen_pt_msi.c

Would it be worthwhile to make it:
- consistent all underscore or all minus ?
- allways xen_ (or xen- depending on the above) ?

--
Sander

On Thu, 6 Dec 2012, Sander Eikelenboom wrote:
> Hi All,
> 
> Yesterday I have tried building and using upstream qemu and seabios.
> Config.mk:
> QEMU_UPSTREAM_URL ?= git://git.qemu.org/qemu.git
> QEMU_UPSTREAM_REVISION ?= master
> 
> SEABIOS_UPSTREAM_URL ?= git://git.qemu.org/seabios.git
> SEABIOS_UPSTREAM_TAG ?= master
> 
> And i''m happy to say that it works quite ok, even with (secondary
pci) pci-passthrough ( using an ATI gfx adapter and windows7 as guest os) :-).
> 
> But it seems to have an issue with a USB controller which is trying to use
msi-X interrupts, which makes xl dmesg report:
> (XEN) [2012-12-06 16:07:24] vmsi.c:108:d32767 Unsupported delivery mode 3
> and when "pci_msitranslate=0" is set the error still occurs, only
this time the correct domain number is reported, instead of the 32767.
> 
> 
> However, when trying to debug, i noticed although making a debug build
(make debug=y && make debug=y install), qemu-dm-<guestname>.log
stays almost empty.
> It seems all the defines related to debugging are not set.
> 
> - Would it be appropriated to enable them all when making a debug build ?
> - Would it be wise to also have some more verbose logging when not running
a debug build ?
Yes and yes
> - And if yes, what would be the nicest way to set the defines ?
My guess is that it would be enough to turn on XEN_PT_LOGGING_ENABLED by
default
> - Should the naming of the debug defines be made more consistent ?
Yes


> 
> When enabling these debug defines by hand:
> 
> xen-all.c
> #define DEBUG_XEN
> 
> xen-mapcache.c
> #define MAPCACHE_DEBUG
> 
> hw/xen-host-pci-device.c
> #define XEN_HOST_PCI_DEVICE_DEBUG
> 
> hw/xen_platform.c
> #define DEBUG_PLATFORM
> 
> hw/xen_pt.h
> #define XEN_PT_LOGGING_ENABLED
> #define XEN_PT_DEBUG_PCI_CONFIG_ACCESS
> 
> I get a lot of compile errors related to wrong types in the debug
printf''s.
That''s really bad. I would like upstream QEMU to have the same level of
logging as qemu-xen-traditional by default. And they should compile.

> Another thing that occurred to me was that the file naming doesn''t
seem to be overly consistent:
> 
> xen-all.c
> xen-mapcache.c
> xen-mapcache.h
> xen-stub.c
> xen_apic.c
> hw/xen_backend.c
> hw/xen_backend.h
> hw/xen_blkif.h
> hw/xen_common.h
> hw/xen_console.c
> hw/xen_devconfig.c
> hw/xen_disk.c
> hw/xen_domainbuild.c
> hw/xen_domainbuild.h
> hw/xenfb.c
> hw/xen.h
> hw/xen-host-pci-device.c
> hw/xen-host-pci-device.h
> hw/xen_machine_pv.c
> hw/xen_nic.c
> hw/xen_platform.c
> hw/xen_pt.c
> hw/xen_pt_config_init.c
> hw/xen_pt.h
> hw/xen_pt_msi.c
> 
> Would it be worthwhile to make it:
> - consistent all underscore or all minus ?
> - allways xen_ (or xen- depending on the above) ?
Yes, definitely.
Given that the development window for QEMU 1.4 has just opened might
even be the right time to make these changes.

Are you volunteering? :)

Friday, December 7, 2012, 6:01:43 PM, you wrote:
> On Thu, 6 Dec 2012, Sander Eikelenboom wrote:
>> Hi All,
>> 
>> Yesterday I have tried building and using upstream qemu and seabios.
>> Config.mk:
>> QEMU_UPSTREAM_URL ?= git://git.qemu.org/qemu.git
>> QEMU_UPSTREAM_REVISION ?= master
>> 
>> SEABIOS_UPSTREAM_URL ?= git://git.qemu.org/seabios.git
>> SEABIOS_UPSTREAM_TAG ?= master
>> 
>> And i''m happy to say that it works quite ok, even with
(secondary pci) pci-passthrough ( using an ATI gfx adapter and windows7 as guest
os) :-).
>> 
>> But it seems to have an issue with a USB controller which is trying to
use msi-X interrupts, which makes xl dmesg report:
>> (XEN) [2012-12-06 16:07:24] vmsi.c:108:d32767 Unsupported delivery mode
3
>> and when "pci_msitranslate=0" is set the error still occurs,
only this time the correct domain number is reported, instead of the 32767.
>> 
>> 
>> However, when trying to debug, i noticed although making a debug build
(make debug=y && make debug=y install), qemu-dm-<guestname>.log
stays almost empty.
>> It seems all the defines related to debugging are not set.
>> 
>> - Would it be appropriated to enable them all when making a debug build
?
>> - Would it be wise to also have some more verbose logging when not
running a debug build ?
> Yes and yes
>> - And if yes, what would be the nicest way to set the defines ?
> My guess is that it would be enough to turn on XEN_PT_LOGGING_ENABLED by
> default
>> - Should the naming of the debug defines be made more consistent ?
> Yes

>> 
>> When enabling these debug defines by hand:
>> 
>> xen-all.c
>> #define DEBUG_XEN
>> 
>> xen-mapcache.c
>> #define MAPCACHE_DEBUG
>> 
>> hw/xen-host-pci-device.c
>> #define XEN_HOST_PCI_DEVICE_DEBUG
>> 
>> hw/xen_platform.c
>> #define DEBUG_PLATFORM
>> 
>> hw/xen_pt.h
>> #define XEN_PT_LOGGING_ENABLED
>> #define XEN_PT_DEBUG_PCI_CONFIG_ACCESS
>> 
>> I get a lot of compile errors related to wrong types in the debug
printf''s.
> That''s really bad. I would like upstream QEMU to have the same
level of
> logging as qemu-xen-traditional by default. And they should compile.
>> Another thing that occurred to me was that the file naming
doesn''t seem to be overly consistent:
>> 
>> xen-all.c
>> xen-mapcache.c
>> xen-mapcache.h
>> xen-stub.c
>> xen_apic.c
>> hw/xen_backend.c
>> hw/xen_backend.h
>> hw/xen_blkif.h
>> hw/xen_common.h
>> hw/xen_console.c
>> hw/xen_devconfig.c
>> hw/xen_disk.c
>> hw/xen_domainbuild.c
>> hw/xen_domainbuild.h
>> hw/xenfb.c
>> hw/xen.h
>> hw/xen-host-pci-device.c
>> hw/xen-host-pci-device.h
>> hw/xen_machine_pv.c
>> hw/xen_nic.c
>> hw/xen_platform.c
>> hw/xen_pt.c
>> hw/xen_pt_config_init.c
>> hw/xen_pt.h
>> hw/xen_pt_msi.c
>> 
>> Would it be worthwhile to make it:
>> - consistent all underscore or all minus ?
>> - allways xen_ (or xen- depending on the above) ?
> Yes, definitely.
> Given that the development window for QEMU 1.4 has just opened might
> even be the right time to make these changes.
> Are you volunteering? :)
Erhmm yes i think i should be able to accomplish this :-)
And yes i did notice the 1.3 release :-)

Patches would be directly against the qemu-upstream git-tree, first round to
xen-devel and when acked send to qemu-list ?


For the filerenaming, the rest of the qemu sources seems to be mixed, but i
think it would be more neat for the xen part to stick to one of the two .. but
which one would be prefered ?
1. all underscores
2. all minus

--
Sander

On Fri, 7 Dec 2012, Sander Eikelenboom wrote:
> Friday, December 7, 2012, 6:01:43 PM, you wrote:
> 
> > On Thu, 6 Dec 2012, Sander Eikelenboom wrote:
> >> Hi All,
> >> 
> >> Yesterday I have tried building and using upstream qemu and
seabios.
> >> Config.mk:
> >> QEMU_UPSTREAM_URL ?= git://git.qemu.org/qemu.git
> >> QEMU_UPSTREAM_REVISION ?= master
> >> 
> >> SEABIOS_UPSTREAM_URL ?= git://git.qemu.org/seabios.git
> >> SEABIOS_UPSTREAM_TAG ?= master
> >> 
> >> And i''m happy to say that it works quite ok, even with
(secondary pci) pci-passthrough ( using an ATI gfx adapter and windows7 as guest
os) :-).
> >> 
> >> But it seems to have an issue with a USB controller which is
trying to use msi-X interrupts, which makes xl dmesg report:
> >> (XEN) [2012-12-06 16:07:24] vmsi.c:108:d32767 Unsupported delivery
mode 3
> >> and when "pci_msitranslate=0" is set the error still
occurs, only this time the correct domain number is reported, instead of the
32767.
> >> 
> >> 
> >> However, when trying to debug, i noticed although making a debug
build (make debug=y && make debug=y install),
qemu-dm-<guestname>.log stays almost empty.
> >> It seems all the defines related to debugging are not set.
> >> 
> >> - Would it be appropriated to enable them all when making a debug
build ?
> >> - Would it be wise to also have some more verbose logging when not
running a debug build ?
> 
> > Yes and yes
> 
> >> - And if yes, what would be the nicest way to set the defines ?
> 
> > My guess is that it would be enough to turn on XEN_PT_LOGGING_ENABLED
by
> > default
> 
> >> - Should the naming of the debug defines be made more consistent ?
> 
> > Yes
> 
> 
> 
> >> 
> >> When enabling these debug defines by hand:
> >> 
> >> xen-all.c
> >> #define DEBUG_XEN
> >> 
> >> xen-mapcache.c
> >> #define MAPCACHE_DEBUG
> >> 
> >> hw/xen-host-pci-device.c
> >> #define XEN_HOST_PCI_DEVICE_DEBUG
> >> 
> >> hw/xen_platform.c
> >> #define DEBUG_PLATFORM
> >> 
> >> hw/xen_pt.h
> >> #define XEN_PT_LOGGING_ENABLED
> >> #define XEN_PT_DEBUG_PCI_CONFIG_ACCESS
> >> 
> >> I get a lot of compile errors related to wrong types in the debug
printf''s.
> 
> > That''s really bad. I would like upstream QEMU to have the
same level of
> > logging as qemu-xen-traditional by default. And they should compile.
> 
> 
> >> Another thing that occurred to me was that the file naming
doesn''t seem to be overly consistent:
> >> 
> >> xen-all.c
> >> xen-mapcache.c
> >> xen-mapcache.h
> >> xen-stub.c
> >> xen_apic.c
> >> hw/xen_backend.c
> >> hw/xen_backend.h
> >> hw/xen_blkif.h
> >> hw/xen_common.h
> >> hw/xen_console.c
> >> hw/xen_devconfig.c
> >> hw/xen_disk.c
> >> hw/xen_domainbuild.c
> >> hw/xen_domainbuild.h
> >> hw/xenfb.c
> >> hw/xen.h
> >> hw/xen-host-pci-device.c
> >> hw/xen-host-pci-device.h
> >> hw/xen_machine_pv.c
> >> hw/xen_nic.c
> >> hw/xen_platform.c
> >> hw/xen_pt.c
> >> hw/xen_pt_config_init.c
> >> hw/xen_pt.h
> >> hw/xen_pt_msi.c
> >> 
> >> Would it be worthwhile to make it:
> >> - consistent all underscore or all minus ?
> >> - allways xen_ (or xen- depending on the above) ?
> 
> > Yes, definitely.
> > Given that the development window for QEMU 1.4 has just opened might
> > even be the right time to make these changes.
> 
> > Are you volunteering? :)
> 
> Erhmm yes i think i should be able to accomplish this :-)
> And yes i did notice the 1.3 release :-)
> 
> Patches would be directly against the qemu-upstream git-tree, first round
to xen-devel and when acked send to qemu-list ?
It is best to CC qemu-devel from the start, they are used to high levels
of traffic anyway ;)
> For the filerenaming, the rest of the qemu sources seems to be mixed, but i
think it would be more neat for the xen part to stick to one of the two .. but
which one would be prefered ?
> 1. all underscores
> 2. all minus
I would go for 1.
However I would keep the renaming patch separate from the others,
because it could start a flame war between underscores supporters
and minuses supporters :)

The other changes (better default debug levels, working debugs, debug
functions naming) should all be non-controversial.

Monday, December 10, 2012, 1:59:51 PM, you wrote:
> On Fri, 7 Dec 2012, Sander Eikelenboom wrote:
>> Friday, December 7, 2012, 6:01:43 PM, you wrote:
>> 
>> > On Thu, 6 Dec 2012, Sander Eikelenboom wrote:
>> >> Hi All,
>> >> 
>> >> Yesterday I have tried building and using upstream qemu and
seabios.
>> >> Config.mk:
>> >> QEMU_UPSTREAM_URL ?= git://git.qemu.org/qemu.git
>> >> QEMU_UPSTREAM_REVISION ?= master
>> >> 
>> >> SEABIOS_UPSTREAM_URL ?= git://git.qemu.org/seabios.git
>> >> SEABIOS_UPSTREAM_TAG ?= master
>> >> 
>> >> And i''m happy to say that it works quite ok, even
with (secondary pci) pci-passthrough ( using an ATI gfx adapter and windows7 as
guest os) :-).
>> >> 
>> >> But it seems to have an issue with a USB controller which is
trying to use msi-X interrupts, which makes xl dmesg report:
>> >> (XEN) [2012-12-06 16:07:24] vmsi.c:108:d32767 Unsupported
delivery mode 3
>> >> and when "pci_msitranslate=0" is set the error still
occurs, only this time the correct domain number is reported, instead of the
32767.
>> >> 
>> >> 
>> >> However, when trying to debug, i noticed although making a
debug build (make debug=y && make debug=y install),
qemu-dm-<guestname>.log stays almost empty.
>> >> It seems all the defines related to debugging are not set.
>> >> 
>> >> - Would it be appropriated to enable them all when making a
debug build ?
>> >> - Would it be wise to also have some more verbose logging when
not running a debug build ?
>> 
>> > Yes and yes
>> 
>> >> - And if yes, what would be the nicest way to set the defines
?
>> 
>> > My guess is that it would be enough to turn on
XEN_PT_LOGGING_ENABLED by
>> > default
>> 
>> >> - Should the naming of the debug defines be made more
consistent ?
>> 
>> > Yes
>> 
>> 
>> 
>> >> 
>> >> When enabling these debug defines by hand:
>> >> 
>> >> xen-all.c
>> >> #define DEBUG_XEN
>> >> 
>> >> xen-mapcache.c
>> >> #define MAPCACHE_DEBUG
>> >> 
>> >> hw/xen-host-pci-device.c
>> >> #define XEN_HOST_PCI_DEVICE_DEBUG
>> >> 
>> >> hw/xen_platform.c
>> >> #define DEBUG_PLATFORM
>> >> 
>> >> hw/xen_pt.h
>> >> #define XEN_PT_LOGGING_ENABLED
>> >> #define XEN_PT_DEBUG_PCI_CONFIG_ACCESS
>> >> 
>> >> I get a lot of compile errors related to wrong types in the
debug printf''s.
>> 
>> > That''s really bad. I would like upstream QEMU to have the
same level of
>> > logging as qemu-xen-traditional by default. And they should
compile.
>> 
>> 
>> >> Another thing that occurred to me was that the file naming
doesn''t seem to be overly consistent:
>> >> 
>> >> xen-all.c
>> >> xen-mapcache.c
>> >> xen-mapcache.h
>> >> xen-stub.c
>> >> xen_apic.c
>> >> hw/xen_backend.c
>> >> hw/xen_backend.h
>> >> hw/xen_blkif.h
>> >> hw/xen_common.h
>> >> hw/xen_console.c
>> >> hw/xen_devconfig.c
>> >> hw/xen_disk.c
>> >> hw/xen_domainbuild.c
>> >> hw/xen_domainbuild.h
>> >> hw/xenfb.c
>> >> hw/xen.h
>> >> hw/xen-host-pci-device.c
>> >> hw/xen-host-pci-device.h
>> >> hw/xen_machine_pv.c
>> >> hw/xen_nic.c
>> >> hw/xen_platform.c
>> >> hw/xen_pt.c
>> >> hw/xen_pt_config_init.c
>> >> hw/xen_pt.h
>> >> hw/xen_pt_msi.c
>> >> 
>> >> Would it be worthwhile to make it:
>> >> - consistent all underscore or all minus ?
>> >> - allways xen_ (or xen- depending on the above) ?
>> 
>> > Yes, definitely.
>> > Given that the development window for QEMU 1.4 has just opened
might
>> > even be the right time to make these changes.
>> 
>> > Are you volunteering? :)
>> 
>> Erhmm yes i think i should be able to accomplish this :-)
>> And yes i did notice the 1.3 release :-)
>> 
>> Patches would be directly against the qemu-upstream git-tree, first
round to xen-devel and when acked send to qemu-list ?
> It is best to CC qemu-devel from the start, they are used to high levels
> of traffic anyway ;)
Ok
>> For the filerenaming, the rest of the qemu sources seems to be mixed,
but i think it would be more neat for the xen part to stick to one of the two ..
but which one would be prefered ?
>> 1. all underscores
>> 2. all minus
> I would go for 1.
> However I would keep the renaming patch separate from the others,
> because it could start a flame war between underscores supporters
> and minuses supporters :)
Ok i thought about that :-)
> The other changes (better default debug levels, working debugs, debug
> functions naming) should all be non-controversial.
Ok will send those first