Re: [libvirt] [PATCH v3 2/4] conf: add support for panic device
On 12/09/2013 04:11 AM, Hu Tao wrote: ...snip... +static bool +virDomainPanicCheckABIStability(virDomainPanicDefPtr src, +virDomainPanicDefPtr dst) +{ +return virDomainDeviceInfoCheckABIStability(src-info, dst-info); +} + These changes have resulted in a libvirtd crash during the virt-test 'snapshot_edit' checks: Thread 1 (Thread 0x7f5332837700 (LWP 10964)): #0 virDomainDeviceInfoCheckABIStability (src=optimized out, dst=optimized out) at conf/domain_conf.c:13007 #1 0x7f534211bbc7 in virDomainPanicCheckABIStability (dst=optimized out, src=optimized out) at conf/domain_conf.c:13712 #2 virDomainDefCheckABIStability (src=optimized out, dst=optimized out) at conf/domain_conf.c:14056 #3 0x7f53421340cb in virDomainSnapshotRedefinePrep (domain=domain@entry=0x7f5357c0, vm=vm@entry=0x7f5336d0, defptr=defptr@entry=0x7f5332836978, snap=snap@entry=0x7f5332836970, update_current=update_current@entry=0x7f5332836962, flags=flags@entry=1) at conf/snapshot_conf.c:1230 #4 0x7f5329e4356c in qemuDomainSnapshotCreateXML (domain=0x7f5357c0, xmlDesc=optimized out, flags=1) at qemu/qemu_driver.c:12719 #5 0x7f53421a0738 in virDomainSnapshotCreateXML (domain=domain@entry=0x7f5357c0, xmlDesc=0x7f5381d0 domainsnapshot\n namesnap2/name\n descriptionnew-desc/description\n staterunning/state\n parent\n namesnap1/name\n /parent\n creationTime1387487268/creationTime\n memory s..., flags=1) at libvirt.c:19695 #6 0x7f5342bc85b7 in remoteDispatchDomainSnapshotCreateXML (server=optimized out, msg=optimized out, ret=0x7f533c80, args=0x7f533c40, rerr=0x7f5332836c80, client=optimized out) at remote_dispatch.h:8223 #7 remoteDispatchDomainSnapshotCreateXMLHelper (server=optimized out, client=optimized out, msg=optimized out, rerr=0x7f5332836c80, args=0x7f533c40, ret=0x7f533c80) at remote_dispatch.h:8199 #8 0x7f53421ede3a in virNetServerProgramDispatchCall (msg=0x7f5344c1acc0, client=0x7f5344c1aa80, server=0x7f5344c01570, prog=0x7f5344c15940) at rpc/virnetserverprogram.c:435 #9 virNetServerProgramDispatch (prog=0x7f5344c15940, server=server@entry=0x7f5344c01570, client=0x7f5344c1aa80, msg=0x7f5344c1acc0) at rpc/virnetserverprogram.c:305 #10 0x7f53421e7c98 in virNetServerProcessMsg (msg=optimized out, prog=optimized out, client=optimized out, srv=0x7f5344c01570) at rpc/virnetserver.c:165 ... (gdb) up 3 (gdb) print *other-def-dom $2 = {virtType = 2, id = -1, uuid = \210Γ;\254e@\212\245\034\032h$\230\030\264, name = 0x7f53201bdb60 virt-tests-vm1, title = 0x0, ... seclabels = 0x7f53201fe650, watchdog = 0x0, memballoon = 0x7f5320201d10, nvram = 0x0, tpm = 0x0, cpu = 0x0, sysinfo = 0x0, redirfilter = 0x0, rng = 0x0, panic = 0x0, namespaceData = 0x0, ns = {parse = 0x7f5329dd2770 qemuDomainDefNamespaceParse, ... (gdb) print *def-dom $3 = {virtType = 2, id = -1, uuid = \210Γ;\254e@\212\245\034\032h$\230\030\264, name = 0x7f53fdf0 virt-tests-vm1, title = 0x0, ... rng = 0x0, panic = 0x0, namespaceData = 0x0, ns = {parse = 0x7f5329dd2770 qemuDomainDefNamespaceParse, Unlike the virDomainRNGDefCheckABIStability() API, the new virDomainPanicCheckABIStability() has no checks for !src !dst I'll put together a patch for this, but figured I'd ask now if there were checks that should also be made in the PanicCheck API... John /* This compares two configurations and looks for any differences * which will affect the guest ABI. This is primarily to allow @@ -13930,6 +13985,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainRNGDefCheckABIStability(src-rng, dst-rng)) return false; +if (!virDomainPanicCheckABIStability(src-panic, dst-panic)) +return false; + return true; } @@ -15776,6 +15834,16 @@ virDomainWatchdogDefFormat(virBufferPtr buf, return 0; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] conf: add support for panic device
On 12/20/2013 06:59 AM, John Ferlan wrote: On 12/09/2013 04:11 AM, Hu Tao wrote: ...snip... +static bool +virDomainPanicCheckABIStability(virDomainPanicDefPtr src, +virDomainPanicDefPtr dst) +{ +return virDomainDeviceInfoCheckABIStability(src-info, dst-info); Unlike the virDomainRNGDefCheckABIStability() API, the new virDomainPanicCheckABIStability() has no checks for !src !dst Yay for automated regression testing catching something! And sorry that I missed this in my initial review. Yes, adding or removing the panic device is an ABI incompatibility. I'll put together a patch for this, but figured I'd ask now if there were checks that should also be made in the PanicCheck API... Not really - a panic device has only two properties: existence, and address (the existence check is missing, which caused the core; but the address check is already there). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] conf: add support for panic device
On Fri, Dec 20, 2013 at 07:11:00AM -0700, Eric Blake wrote: On 12/20/2013 06:59 AM, John Ferlan wrote: On 12/09/2013 04:11 AM, Hu Tao wrote: ...snip... +static bool +virDomainPanicCheckABIStability(virDomainPanicDefPtr src, +virDomainPanicDefPtr dst) +{ +return virDomainDeviceInfoCheckABIStability(src-info, dst-info); Unlike the virDomainRNGDefCheckABIStability() API, the new virDomainPanicCheckABIStability() has no checks for !src !dst Yay for automated regression testing catching something! And sorry that I missed this in my initial review. Yes, adding or removing the panic device is an ABI incompatibility. Indeed. The ABI stability checks are something we really ought to be able to have our unit tests do in fact. At the very least we could feed in every single XML file we have in the test suite as both the src dst params, to serve as an identity test. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] conf: add support for panic device
On Mon, Dec 09, 2013 at 05:11:14PM +0800, Hu Tao wrote: panic device is a device that enables libvirt to receive notification of guest panic event. --- docs/formatdomain.html.in | 28 + docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c| 72 +++ src/conf/domain_conf.h| 9 ++ 4 files changed, 119 insertions(+) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] conf: add support for panic device
On 12/12/2013 05:08 AM, Daniel P. Berrange wrote: On Mon, Dec 09, 2013 at 05:11:14PM +0800, Hu Tao wrote: panic device is a device that enables libvirt to receive notification of guest panic event. --- docs/formatdomain.html.in | 28 + docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c| 72 +++ src/conf/domain_conf.h| 9 ++ domain_conf.h gave me a conflict to based on tree activity in the meantime, but I got that fixed. 4 files changed, 119 insertions(+) ACK Will push with the rest of the series. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list