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
>
> [...]
>> +
>> + /* Needed? A way for users to list a disk and explicitly mark it
>> + * as not participating, and then output shows all disks rather
>> + * than just active disks */
>> +#if 0
>> + backup = virXMLPropString(node, "backup");
>> + if (backup) {
>> + def->type = virDomainCheckpointTypeFromString(checkpoint);
>> + if (def->type <= 0) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unknown disk checkpoint setting '%s'"),
>> + checkpoint);
>> + goto cleanup;
>> + }
>> + }
>> +#endif
>
> Some leftovers?
Yeah, I need to patch that up. For the latest version of the XML
documentation, I added <disk checkpoint='no'> with a comment that
anything other than 'no' is informative, then wired the RNG to let it
actually be a state machine ('begin', 'exported', 'ready', ...) tracking
the various phases of which QMP commands have been issued (and therefore
which commands have to be done in reverse for cleanup regardless of
whether cleanup is full after success or partial after early failure).
But I haven't finished wiring that through checkpoint_conf.c or through
the later patches; at the moment, the later patches have some rather
awkward hacks on checking if def->disks[i]->src is NULL (no disk image
associated, so must be a disk not involved in the snapshot), vs. a
separate state in def->disks[i]->state; but if I can properly unify
that, I think it will be easier to manage. May be followup patches.
>
>> +
>> + if ((type = virXMLPropString(node, "type"))) {
>> + if ((def->store->type = virStorageTypeFromString(type)) <= 0 ||
>> + def->store->type == VIR_STORAGE_TYPE_VOLUME ||
>> + def->store->type == VIR_STORAGE_TYPE_DIR) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("unknown disk backup type '%s'"), type);
>> + goto cleanup;
>> + }
>> + } else {
>> + def->store->type = VIR_STORAGE_TYPE_FILE;
>> + }
>> +
>> + 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.
>
> Same applies for the formatter below.
>
> (yes, the original implementation is suboptimal as I've put
> nodeformat/nodestorage into virStorageSource, but for domain stuff it's
> never formatted using the code in src/conf)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
