Re: [PATCH v7 14/23] backup: Parse and output backup XML

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 27, 2019 at 13:18:17 +0100, Peter Krempa wrote:
> On Wed, Mar 27, 2019 at 07:14:09 -0500, Eric Blake wrote:
> > On 3/27/19 7:05 AM, Peter Krempa wrote:
> > > On Wed, Mar 27, 2019 at 05:10:45 -0500, Eric Blake wrote:
> > >> Accept XML describing a generic block job, and output it again as
> > >> needed. This may still need a few tweaks to match the documented XML
> > >> and RNG schema.
> > >>
> > >> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> > >> ---
> > >>  src/conf/backup_conf.h   |  96 +++++++
> > >>  src/conf/virconftypes.h  |   3 +
> > >>  src/conf/Makefile.inc.am |   2 +
> > >>  src/conf/backup_conf.c   | 544 +++++++++++++++++++++++++++++++++++++++
> > >>  src/libvirt_private.syms |   8 +-
> > >>  5 files changed, 652 insertions(+), 1 deletion(-)
> > >>  create mode 100644 src/conf/backup_conf.h
> > >>  create mode 100644 src/conf/backup_conf.c
> 
> [...]
> 
> > >> +    if ((cur = virXPathNode(push ? "./target" : "./scratch", ctxt)) &&
> > >> +        virDomainDiskSourceParse(cur, ctxt, def->store, 0, xmlopt) < 0)
> > >> +        goto cleanup;
> > >> +
> > >> +    if (internal) {
> > >> +        int detected;
> > >> +        if (virXPathInt("string(./node/@detected)", ctxt, &detected) < 0)
> > >> +            goto cleanup;
> > >> +        def->store->detected = detected;
> > >> +        def->store->nodeformat = virXPathString("string(./node)", ctxt);
> > > 
> > > This should not be public name. Current design of nodenames does not
> > > allow you to depend on the name.
> > > 
> > > In general, nodenames are a qemu-specific thing thus should not be
> > > expressed nor documented in the public XML.
> > 
> > Hence the 'internal' flag; this is ONLY supposed to be output for qemu's
> > internal storage to /var/lib/libvirt/qemu/domain.xml to persist state
> > across libvirtd restarts.  I also think that by the end of my series, I
> > was trying to avoid this ever landing on disk, and instead teaching
> > libvirt to re-parse current node names from QMP on the fly before any
> > operation where the node names are not still in memory from an earlier time.
> 
> Okay, so it's a case of technical debt since all this private stuff will
> later need refactoring to private data callbacks.

One more thing. To avoid the need to always parse this in src/conf/ you
should encapsulate it into <privateData>. That way it will be possible
to refactor it later if we decide decide that it's okay to add the
parser/formatter here.


Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux