Re: [libvirt] [PATCH v3 2/4] conf: add support for panic device

2013-12-20 Thread John Ferlan


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

2013-12-20 Thread Eric Blake
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

2013-12-20 Thread Daniel P. Berrange
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

2013-12-12 Thread Daniel P. Berrange
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

2013-12-12 Thread Eric Blake
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