8:29 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org