On 08/19/2011 09:08 PM, Eric Blake wrote:
I got confused when 'virsh domblkinfo dom disk' required the
path to a disk (which can be ambiguous, since a single file
can back multiple disks), rather than the unambiguous target
device name that I was using in disk snapshots. So, in true
developer fashion, I went for the best of both worlds - all
interfaces that operate on a disk (aka block) now accept
either the target name or the unambiguous path to the backing
file used by the disk.
@@ -11284,6 +11312,13 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
def,
disk->file, disk->name);
goto cleanup;
}
+if (STRNEQ(disk->file, def->dom->disks[idx]->dst)) {
+VIR_FREE(disk->file);
+if (!(disk->file = strdup(def->dom->disks[idx]->dst))) {
+virReportOOMError();
+goto cleanup;
+}
+}
I got a NULL deref in testing, and traced it to the use of the wrong
field. Squash this in (I promise to post a clean v3 of the entire
series once I flush out more of these little fixups).
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index aa5f2d3..effc2a3 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -11314,9 +11314,9 @@
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
disk->file, disk->name);
goto cleanup;
}
-if (STRNEQ(disk->file, def->dom->disks[idx]->dst)) {
-VIR_FREE(disk->file);
-if (!(disk->file = strdup(def->dom->disks[idx]->dst))) {
+if (STRNEQ(disk->name, def->dom->disks[idx]->dst)) {
+VIR_FREE(disk->name);
+if (!(disk->name = strdup(def->dom->disks[idx]->dst))) {
virReportOOMError();
goto cleanup;
}
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I got confused when 'virsh domblkinfo dom disk' required the
path to a disk (which can be ambiguous, since a single file
can back multiple disks), rather than the unambiguous target
device name that I was using in disk snapshots. So, in true
developer fashion, I went for the best of both worlds - all
interfaces that operate on a disk (aka block) now accept
either the target name or the unambiguous path to the backing
file used by the disk.
* src/conf/domain_conf.h (virDomainDiskIndexByName): Add
parameter.
(virDomainDiskPathByName): New prototype.
* src/libvirt_private.syms (domain_conf.h): Export it.
* src/conf/domain_conf.c (virDomainDiskIndexByName): Also allow
searching by path, and decide whether ambiguity is okay.
(virDomainDiskPathByName): New function.
(virDomainDiskRemoveByName, virDomainSnapshotAlignDisks): Update
callers.
* src/qemu/qemu_driver.c (qemudDomainBlockPeek)
(qemuDomainAttachDeviceConfig, qemuDomainUpdateDeviceConfig)
(qemuDomainGetBlockInfo, qemuDiskPathToAlias): Likewise.
* src/qemu/qemu_process.c (qemuProcessFindDomainDiskByPath):
Likewise.
* src/libxl/libxl_driver.c (libxlDomainAttachDeviceDiskLive)
(libxlDomainDetachDeviceDiskLive, libxlDomainAttachDeviceConfig)
(libxlDomainUpdateDeviceConfig): Likewise.
* src/uml/uml_driver.c (umlDomainBlockPeek): Likewise.
* src/xen/xend_internal.c (xenDaemonDomainBlockPeek): Likewise.
* docs/formatsnapshot.html.in: Update documentation.
* tools/virsh.pod (domblkstat, domblkinfo): Likewise.
* docs/schemas/domaincommon.rng (diskTarget): Tighten pattern on
disk targets.
* docs/schemas/domainsnapshot.rng (disksnapshot): Update to match.
* tests/domainsnapshotxml2xmlin/disk_snapshot.xml: Update test.
---
docs/formatsnapshot.html.in |7 +-
docs/schemas/domaincommon.rng |7 ++-
docs/schemas/domainsnapshot.rng |5 +-
src/conf/domain_conf.c | 57 ++---
src/conf/domain_conf.h |4 +-
src/libvirt_private.syms|1 +
src/libxl/libxl_driver.c| 11 ++-
src/qemu/qemu_driver.c | 104 ++
src/qemu/qemu_process.c | 11 +--
src/uml/uml_driver.c| 56 ++---
src/xen/xend_internal.c | 12 +--
tests/domainsnapshotxml2xmlin/disk_snapshot.xml |2 +-
tools/virsh.pod |8 ++-
13 files changed, 155 insertions(+), 130 deletions(-)
diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 5aebd72..e476c8f 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -109,8 +109,9 @@
disk
This sub-element describes the snapshot properties of a
specific disk. The attribute name is
-mandatory, and must match the of one of
+mandatory, and must match either the or an unambiguous of one of
the disk
devices specified for the domain at the time of the
snapshot. The attribute snapshot is
@@ -179,7 +180,7 @@
Snapshot of OS install and updates
-
+
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 0af9e0f..e1c6a95 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -770,10 +770,15 @@
+
+
+ (ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+
+
+
-
+
diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
index 671fbe0..0ef0631 100644
--- a/docs/schemas/domainsnapshot.rng
+++ b/docs/schemas/domainsnapshot.rng
@@ -82,7 +82,10 @@
-
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 043e73f..ec75dbd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5516,17 +5516,44 @@ virDomainChrTargetTypeToString(int deviceType,
return type;
}
-int virDomainDiskIndexByName(virDomainDefPtr def, const char *name)
+int
+virDomainDiskIndexByName(virDomainDefPtr def, const char *name,
+ bool allow_ambiguous)
{
virDomainDiskDefPtr vdisk;
int i;
+int candidate = -1;
+/* We prefer the name (it's shorter, required
+ * for all disks, and should be unambiguous), but also support
+ * (if unambiguous). Assume dst if there is
+ * no leading slash, source name otherwise. */
for (i = 0; i < def->ndisks; i++) {
vdisk = def->disks[i];
-if (STREQ(vdisk->dst, na
