Re: [libvirt] [PATCHv4 07/18] blockjob: enhance xml to track mirrors across libvirtd restart

On Mon, Apr 09, 2012 at 21:52:16 -0600, Eric Blake wrote: > QUESTION: should we parse and ignore <mirror> on input, rather than > rejecting it? By rejecting it, I can't add a unit test, since the > unit test framework currently doesn't expose a way to trigger > internal parsing. > > In order to track a block copy job across libvirtd restarts, we > need to save internal XML that tracks the name of the file > holding the mirror. Displaying this name in dumpxml might also > be useful to the user, even if we don't yet have a way to (re-) > start a domain with mirroring enabled up front. This is done > with a new <mirror> sub-element to <disk>, as in: > > <disk type='file' device='disk'> > <driver name='qemu' type='raw'/> > <source file='/var/lib/libvirt/images/original.img'/> > <mirror file='/var/lib/libvirt/images/copy.img' format='qcow2'/> > ... > </disk> However, this would mean, the XML from virDomainGetXMLDesc() cannot be directly used for defining new domain. I think there are two possible ways to go. Either output <mirror> the way you did it and ignore it on input (similarly to what we do with aliases or SELinux labels) or put the mirror element out of domain XML and put it into state XML only.

> @@ -3453,6 +3457,22 @@ virDomainDiskDefParseXML(virCapsPtr caps, > ioeventfd = virXMLPropString(cur, "ioeventfd"); > event_idx = virXMLPropString(cur, "event_idx"); > copy_on_read = virXMLPropString(cur, "copy_on_read"); > + } else if ((mirror == NULL) && > + (xmlStrEqual(cur->name, BAD_CAST "mirror"))) { You could have also removed those extra () when copy&pasting the code :-)

> > + char *mirror; > + char *mirrorFormat; > + bool mirroring; > + > virDomainBlockIoTuneInfo blkdeviotune; > > char *serial; > @@ -2125,6 +2129,7 @@ VIR_ENUM_DECL(virDomainDiskIo) > VIR_ENUM_DECL(virDomainDiskSecretType) > VIR_ENUM_DECL(virDomainDiskSnapshot) > VIR_ENUM_DECL(virDomainDiskTray) > +VIR_ENUM_DECL(virDomainDiskMirrorStage) > VIR_ENUM_DECL(virDomainIoEventFd) > VIR_ENUM_DECL(virDomainVirtioEventIdx) > VIR_ENUM_DECL(virDomainDiskCopyOnRead) This last hunk looks weired. Should it go into another patch perhaps?