[libvirt] [PATCH v8 11/21] backup: Parse and output backup XML

[Eric Blake]

On 4/17/19 9:09 AM, 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 at redhat.com>
> ---

> +/* Align def->disks to domain.  Sort the list of def->disks,
> + * generating storage names using suffix as needed.  Convert paths to
> + * disk targets for uniformity.  Issue an error and return -1 if any
> + * def->disks[n]->name appears more than once or does not map to
> + * dom->disks. */
> +int
> +virDomainBackupAlignDisks(virDomainBackupDefPtr def, virDomainDefPtr dom,
> +                          const char *suffix)
> +{

> +    /* Double check requested disks.  */
> +    for (i = 0; i < def->ndisks; i++) {
> +        virDomainBackupDiskDefPtr disk = &def->disks[i];
> +        int idx = virDomainDiskIndexByName(dom, disk->name, false);

Eyal helped me (finally) diagnose a bug here:

> +
> +        if (idx < 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("no disk named '%s'"), disk->name);
> +            goto cleanup;
> +        }
> +
> +        if (virBitmapIsBitSet(map, idx)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("disk '%s' specified twice"),
> +                           disk->name);
> +            goto cleanup;
> +        }
> +        ignore_value(virBitmapSetBit(map, idx));
> +        disk->idx = idx;
> +
> +        if (STRNEQ(disk->name, dom->disks[idx]->dst)) {
> +            VIR_FREE(disk->name);
> +            if (VIR_STRDUP(disk->name, dom->disks[idx]->dst) < 0)

Most of this loop is using dom->disks[idx], but

> +                goto cleanup;
> +        }
> +        if (disk->store && !disk->store->path) {
> +            virStorageSourceClear(disk->store);
> +            disk->store = NULL;
> +        }
> +        if (virDomainBackupDefAssignStore(disk, dom->disks[i]->src, suffix) < 0)

this is using dom->disks[i], with disastrous results in the
<domainbackup> lists its <disks> element in a different order than the
<domain> did (that is, when i != idx, such as when there is an empty
cdrom in dom->disks[0]).  I've pushed a backup-v8a tag with that single
fix while I still work on polishing my backup-v9 tag.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190706/27598c38/attachment-0001.sig>