On 08/19/2011 03:58 PM, Eric Blake wrote:
Adds an optional element to<domainsnapshot>, which will be used to give user control over external snapshot filenames on input, and specify generated filenames on output.
+ /* Double check requested disks. */ + for (i = 0; i< def->ndisks; i++) { + virDomainSnapshotDiskDefPtr disk =&def->disks[i]; + bool found = false; + + for (j = 0; j< def->dom->ndisks; j++) { + if (STREQ(disk->name, def->dom->disks[j]->dst)) { + int disk_snapshot = def->dom->disks[j]->snapshot;
Rather than open-code my own name lookup, I just noticed virDomainDiskIndexByName. And using that function has another benefit, to come up in my next path - 'virsh domblkinfo domain disk-name' should accept the same set of names as 'virsh snapshot-create' xml (right now, they don't - domblkinfo uses the source path instead of the target device string; what's worse is the source path is not necessarily unique). Squash this in: diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 50a9bb4..043e73f 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -11218,7 +11218,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, int ret = -1; virBitmapPtr map = NULL; int i; - int j; bool inuse; if (!def->dom) { @@ -11247,46 +11246,42 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, /* Double check requested disks. */ for (i = 0; i < def->ndisks; i++) { virDomainSnapshotDiskDefPtr disk = &def->disks[i]; - bool found = false; + int idx = virDomainDiskIndexByName(def->dom, disk->name); + int disk_snapshot; - for (j = 0; j < def->dom->ndisks; j++) { - if (STREQ(disk->name, def->dom->disks[j]->dst)) { - int disk_snapshot = def->dom->disks[j]->snapshot; + if (idx < 0) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("no disk named '%s'"), disk->name); + goto cleanup; + } + disk_snapshot = def->dom->disks[idx]->snapshot; - if (virBitmapGetBit(map, j, &inuse) < 0 || inuse) { - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' specified twice"), - disk->name); - goto cleanup; - } - ignore_value(virBitmapSetBit(map, j)); - disk->index = j; - if (!disk_snapshot) - disk_snapshot = default_snapshot; - if (!disk->snapshot) { - disk->snapshot = disk_snapshot; - } else if (disk_snapshot && require_match && - disk->snapshot != disk_snapshot) { - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' must use snapshot mode " - "'%s'"), disk->name, - virDomainDiskSnapshotTypeToString(disk_snapshot)); - goto cleanup; - } - if (disk->file && - disk->snapshot != VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL) { - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("file '%s' for disk '%s' requires " - "use of external snapshot mode"), - disk->file, disk->name); - goto cleanup; - } - break; - } + if (virBitmapGetBit(map, idx, &inuse) < 0 || inuse) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' specified twice"), + disk->name); + goto cleanup; } - if (!found) { + ignore_value(virBitmapSetBit(map, idx)); + disk->index = idx; + if (!disk_snapshot) + disk_snapshot = default_snapshot; + if (!disk->snapshot) { + disk->snapshot = disk_snapshot; + } else if (disk_snapshot && require_match && + disk->snapshot != disk_snapshot) { + const char *tmp = virDomainDiskSnapshotTypeToString(disk_snapshot); virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("no disk named '%s'"), disk->name); + _("disk '%s' must use snapshot mode '%s'"), + disk->name, tmp); + goto cleanup; + } + if (disk->file && + disk->snapshot != VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("file '%s' for disk '%s' requires " + "use of external snapshot mode"), + disk->file, disk->name); goto cleanup; } } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org