3:59 p.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1142631
Patch 1 was found when adjusting the bz to get the disk by the pool source
instead of directly by file attribute. When using domblklist, the source
wasn't displayed. Followed the model of the iscsi '@name' and added
'@volume'
(eg, <source pool='default' volume='test-1g.img'/>)
Patch 2 started as a simple (hah!) double loop looking for duplicate dst
strings. I chose a separate API called before qemuCheckDiskConfig since
it's more of a "common" check that could be used by other drivers as
opposed
to the perhaps more specific single disk checks made. Besides I didn't want
a ndisk factorial checking algorithm each time a disk was checked for something
that could be checked once.
With the code in place, running make check tripped over a number of existing
tests that duplicated that '<target dev=...' value, so I fixed those. Of
interest for a few was that the test would duplicate the supposedly unique
alias id - those are cleaned up as well as a byproduct of the changes.
John Ferlan (2):
virsh: Adjust domblklist to understand disk volume syntax
disk: Disallow duplicated target 'dev' values
src/conf/domain_conf.c | 29 ++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 3 +++
.../qemuxml2argv-disk-scsi-disk-split.xml | 2 +-
...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml | 2 +-
.../qemuxml2argv-disk-scsi-lun-passthrough.xml | 2 +-
.../qemuxml2argv-disk-source-pool-mode.xml | 2 +-
.../qemuxml2argv-disk-source-pool.xml | 2 +-
.../qemuxml2argv-pci-bridge-many-disks.args | 4 +--
.../qemuxml2argv-pci-bridge-many-disks.xml | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-pci-many.args | 8 +++---
tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml | 4 +--
.../qemuxml2xmlout-disk-source-pool.xml | 2 +-
.../qemuxml2xmlout-pci-bridge-many-disks.xml | 2 +-
tools/virsh-domain-monitor.c | 3 ++-
16 files changed, 52 insertions(+), 17 deletions(-)
--
2.1.0
3:59 p.m.
New subject: [libvirt] [PATCH 1/2] virsh: Adjust domblklist to understand disk volume syntax
A disk using a source pool is listed as having a source '-' in domblklist
because it doesn't check the right XML syntax to find the source.
Add a check for "./source/volume" which is where the "path" (of
sorts)
to the volume name is described.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tools/virsh-domain-monitor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 925eb1b..3c16b3e 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -531,7 +531,8 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
source = virXPathString("string(./source/@file"
"|./source/@dev"
"|./source/@dir"
- "|./source/@name)", ctxt);
+ "|./source/@name"
+ "|./source/@volume)", ctxt);
if (details) {
vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device,
target, source ? source : "-");
--
2.1.0
8:52 a.m.
New subject: [libvirt] [PATCH 1/2] virsh: Adjust domblklist to understand disk volume syntax
On Thu, Feb 26, 2015 at 04:59:02PM -0500, John Ferlan wrote:
A disk using a source pool is listed as having a source '-' in
domblklist
because it doesn't check the right XML syntax to find the source.
Add a check for "./source/volume" which is where the "path" (of
sorts)
to the volume name is described.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tools/virsh-domain-monitor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
ACK
diff --git a/tools/virsh-domain-monitor.c
b/tools/virsh-domain-monitor.c
index 925eb1b..3c16b3e 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -531,7 +531,8 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
source = virXPathString("string(./source/@file"
"|./source/@dev"
"|./source/@dir"
- "|./source/@name)", ctxt);
+ "|./source/@name"
+ "|./source/@volume)", ctxt);
if (details) {
vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device,
target, source ? source : "-");
--
2.1.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
3:59 p.m.
New subject: [libvirt] [PATCH 2/2] disk: Disallow duplicated target 'dev' values
https://bugzilla.redhat.com/show_bug.cgi?id=1142631
This patch resolves a situation where the same "<target
dev='$name'...>"
can be used for multiple disks in the domain.
While the $name is "mostly" advisory regarding the expected order that
the disk is added to the domain and not guaranteed to map to the device
name in the guest OS, it still should be unique enough such that other
domblk* type operations can be performed.
Without the patch, the domblklist will list the same Target twice:
$ virsh domblklist $dom
Target Source
------------------------------------------------
sda /var/lib/libvirt/images/file.qcow2
sda /var/lib/libvirt/images/file.img
Additionally, getting domblkstat, domblkerror, domblkinfo, and other block*
type calls will not be able to reference the second target.
Fortunately, hotplug disallows adding a "third" sda value:
$ qemu-img create -f raw /var/lib/libvirt/images/file2.img 10M
$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sda
error: Failed to attach disk
error: operation failed: target sda already exists
$
BUT, it since 'sdb' doesn't exist one would get the following on the same
hotplug attempt, but changing to use 'sdb' instead of 'sda'
$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sdb
error: Failed to attach disk
error: internal error: unable to execute QEMU command 'device_add': Duplicate ID
'scsi0-0-1' for device
$
Since we cannot fix this issue at parsing time, the best that can be done so
as to not "lose" a domain is to make the check prior to starting the guest
with the results as follows:
$ virsh start $dom
error: Failed to start domain $dom
error: XML error: target 'sda' duplicated for disk sources
'/var/lib/libvirt/images/file.qcow2' and
'/var/lib/libvirt/images/file.img'
$
Running 'make check' found a few more instances in the tests where this
duplicated target dev value was being used. These also exhibited some
duplicated 'id=' values (negating the uniqueness argument of aliases) in
the corresponding .args file and of course the *xmlout version of a few
input XML files.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 29 ++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 3 +++
.../qemuxml2argv-disk-scsi-disk-split.xml | 2 +-
...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml | 2 +-
.../qemuxml2argv-disk-scsi-lun-passthrough.xml | 2 +-
.../qemuxml2argv-disk-source-pool-mode.xml | 2 +-
.../qemuxml2argv-disk-source-pool.xml | 2 +-
.../qemuxml2argv-pci-bridge-many-disks.args | 4 +--
.../qemuxml2argv-pci-bridge-many-disks.xml | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-pci-many.args | 8 +++---
tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml | 4 +--
.../qemuxml2xmlout-disk-source-pool.xml | 2 +-
.../qemuxml2xmlout-pci-bridge-many-disks.xml | 2 +-
15 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d95dd3e..1a51c1d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11676,6 +11676,35 @@ virDomainDiskControllerMatch(int controller_type, int disk_bus)
return false;
}
+/* Return true if there's a duplicate disk[]->dst name for the same bus */
+bool
+virDomainDiskDefDstDuplicates(virDomainDefPtr def)
+{
+ size_t i, j;
+
+ /* optimization */
+ if (def->ndisks <= 1)
+ return false;
+
+ for (i = 0; i < def->ndisks; i++) {
+ for (j = 0; j < def->ndisks; j++) {
+ if (i == j)
+ continue; /* not ourselves */
+ if (def->disks[i]->bus == def->disks[j]->bus &&
+ STREQ(def->disks[i]->dst, def->disks[j]->dst)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("target '%s' duplicated for disk sources
"
+ "'%s' and '%s'"),
+ def->disks[i]->dst,
+ NULLSTR(virDomainDiskGetSource(def->disks[i])),
+ NULLSTR(virDomainDiskGetSource(def->disks[j])));
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
int
virDomainDiskIndexByAddress(virDomainDefPtr def,
virDevicePCIAddressPtr pci_address,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 02ddd93..ee95f19 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2574,6 +2574,7 @@ int virDomainEmulatorPinDel(virDomainDefPtr def);
void virDomainRNGDefFree(virDomainRNGDefPtr def);
+bool virDomainDiskDefDstDuplicates(virDomainDefPtr def);
int virDomainDiskIndexByAddress(virDomainDefPtr def,
virDevicePCIAddressPtr pci_controller,
unsigned int bus, unsigned int target,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ba05cc6..a9228a8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -224,6 +224,7 @@ virDomainDiskBusTypeToString;
virDomainDiskCacheTypeFromString;
virDomainDiskCacheTypeToString;
virDomainDiskDefAssignAddress;
+virDomainDiskDefDstDuplicates;
virDomainDiskDefForeachPath;
virDomainDiskDefFree;
virDomainDiskDefNew;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 24b2ad9..7913106 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3882,6 +3882,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
int controllerModel;
+ if (virDomainDiskDefDstDuplicates(def))
+ goto error;
+
if (qemuCheckDiskConfig(disk) < 0)
goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-split.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-split.xml
index 4402f3e..20b6f28 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-split.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-split.xml
@@ -21,7 +21,7 @@
</disk>
<disk type='block' device='cdrom'>
<source dev='/dev/HostVG/QEMUGuest2'/>
- <target dev='sda' bus='scsi'/>
+ <target dev='sdb' bus='scsi'/>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
<disk type='file' device='disk'>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml
index 7cf57ec..4dbbd29 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml
@@ -21,7 +21,7 @@
</disk>
<disk type='block' device='lun' sgio='filtered'>
<source dev='/dev/HostVG/QEMUGuest2'/>
- <target dev='hda' bus='scsi'/>
+ <target dev='hdb' bus='scsi'/>
<address type='drive' controller='0' bus='0'
target='1' unit='1'/>
</disk>
<controller type='scsi' index='0'
model='virtio-scsi'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough.xml
index 8111ea3..3858ede 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough.xml
@@ -21,7 +21,7 @@
</disk>
<disk type='block' device='lun'>
<source dev='/dev/HostVG/QEMUGuest2'/>
- <target dev='hda' bus='scsi'/>
+ <target dev='hdb' bus='scsi'/>
<address type='drive' controller='0' bus='0'
target='1' unit='1'/>
</disk>
<controller type='scsi' index='0'
model='virtio-scsi'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
index 9f90293..c791717 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
@@ -30,7 +30,7 @@
<label>system_u:system_r:public_content_t:s0</label>
</seclabel>
</source>
- <target dev='hda' bus='ide'/>
+ <target dev='hdb' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='0'
target='0' unit='2'/>
</disk>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
index 95d5be2..ef095a0 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
@@ -34,7 +34,7 @@
</disk>
<disk type='file' device='disk'>
<source file='/tmp/idedisk.img'/>
- <target dev='hdc' bus='ide'/>
+ <target dev='hdd' bus='ide'/>
<address type='drive' controller='0' bus='0'
target='0' unit='2'/>
</disk>
<controller type='usb' index='0'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.args
b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.args
index 893eaa1..fcd9692 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.args
@@ -8,8 +8,8 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-device pci-bridge,chassis_nr=3,id=pci.3,bus=pci.0,addr=0x5 \
-usb -drive file=/var/lib/libvirt/images/test.img,if=none,id=drive-virtio-disk0 \
-device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio-disk0 \
--drive file=/var/lib/libvirt/images/disk-a-a.img,if=none,id=drive-virtio-disk0 \
--device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0 \
+-drive file=/var/lib/libvirt/images/disk-a-a.img,if=none,id=drive-virtio-disk26 \
+-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk26,id=virtio-disk26 \
-drive file=/var/lib/libvirt/images/disk-a-b.img,if=none,id=drive-virtio-disk27 \
-device virtio-blk-pci,bus=pci.0,addr=0x8,drive=drive-virtio-disk27,id=virtio-disk27 \
-drive file=/var/lib/libvirt/images/disk-a-c.img,if=none,id=drive-virtio-disk28 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.xml
index 1f3ad6e..bc42921 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-many-disks.xml
@@ -29,7 +29,7 @@
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/disk-a-a.img'/>
- <target dev='vda' bus='virtio'/>
+ <target dev='vdaa' bus='virtio'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-many.args
b/tests/qemuxml2argvdata/qemuxml2argv-pci-many.args
index 9551c91..0f33065 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pci-many.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-many.args
@@ -6,10 +6,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
-drive file=/var/lib/libvirt/images/test.img,if=none,id=drive-virtio-disk0 \
-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 \
--drive file=/var/lib/libvirt/images/test1.img,if=none,id=drive-virtio-disk0 \
--device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0 \
--drive file=/var/lib/libvirt/images/disk-a-a.img,if=none,id=drive-virtio-disk0 \
--device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \
+-drive file=/var/lib/libvirt/images/test1.img,if=none,id=drive-virtio-disk1 \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,id=virtio-disk1 \
+-drive file=/var/lib/libvirt/images/disk-a-a.img,if=none,id=drive-virtio-disk26 \
+-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk26,id=virtio-disk26 \
-drive file=/var/lib/libvirt/images/disk-a-b.img,if=none,id=drive-virtio-disk27 \
-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk27,id=virtio-disk27 \
-drive file=/var/lib/libvirt/images/disk-a-c.img,if=none,id=drive-virtio-disk28 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml
index 400470e..b3c9ab1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml
@@ -42,13 +42,13 @@
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/test1.img'/>
- <target dev='vda' bus='virtio'/>
+ <target dev='vdb' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/disk-a-a.img'/>
- <target dev='vda' bus='virtio'/>
+ <target dev='vdaa' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05' function='0x0'/>
</disk>
<disk type='file' device='disk'>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml
index e96f76e..31e4928 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml
@@ -32,7 +32,7 @@
</disk>
<disk type='file' device='disk'>
<source file='/tmp/idedisk.img'/>
- <target dev='hdc' bus='ide'/>
+ <target dev='hdd' bus='ide'/>
<address type='drive' controller='0' bus='0'
target='0' unit='2'/>
</disk>
<controller type='usb' index='0'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml
index d49f5f4..060fe87 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml
@@ -30,7 +30,7 @@
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/disk-a-a.img'/>
- <target dev='vda' bus='virtio'/>
+ <target dev='vdaa' bus='virtio'/>
</disk>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
--
2.1.0
9:13 a.m.
New subject: [libvirt] [PATCH 2/2] disk: Disallow duplicated target 'dev' values
On Thu, Feb 26, 2015 at 04:59:03PM -0500, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1142631
This patch resolves a situation where the same "<target
dev='$name'...>"
can be used for multiple disks in the domain.
While the $name is "mostly" advisory regarding the expected order that
the disk is added to the domain and not guaranteed to map to the device
name in the guest OS, it still should be unique enough such that other
domblk* type operations can be performed.
Without the patch, the domblklist will list the same Target twice:
$ virsh domblklist $dom
Target Source
------------------------------------------------
sda /var/lib/libvirt/images/file.qcow2
sda /var/lib/libvirt/images/file.img
Additionally, getting domblkstat, domblkerror, domblkinfo, and other block*
type calls will not be able to reference the second target.
Fortunately, hotplug disallows adding a "third" sda value:
$ qemu-img create -f raw /var/lib/libvirt/images/file2.img 10M
$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sda
error: Failed to attach disk
error: operation failed: target sda already exists
$
BUT, it since 'sdb' doesn't exist one would get the following on the same
hotplug attempt, but changing to use 'sdb' instead of 'sda'
$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sdb
error: Failed to attach disk
error: internal error: unable to execute QEMU command 'device_add': Duplicate ID
'scsi0-0-1' for device
$
Since we cannot fix this issue at parsing time, the best that can be done so
as to not "lose" a domain is to make the check prior to starting the guest
with the results as follows:
$ virsh start $dom
error: Failed to start domain $dom
error: XML error: target 'sda' duplicated for disk sources
'/var/lib/libvirt/images/file.qcow2' and
'/var/lib/libvirt/images/file.img'
$
Running 'make check' found a few more instances in the tests where this
duplicated target dev value was being used. These also exhibited some
duplicated 'id=' values (negating the uniqueness argument of aliases) in
the corresponding .args file and of course the *xmlout version of a few
input XML files.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 29 ++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 3 +++
.../qemuxml2argv-disk-scsi-disk-split.xml | 2 +-
...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml | 2 +-
.../qemuxml2argv-disk-scsi-lun-passthrough.xml | 2 +-
.../qemuxml2argv-disk-source-pool-mode.xml | 2 +-
.../qemuxml2argv-disk-source-pool.xml | 2 +-
.../qemuxml2argv-pci-bridge-many-disks.args | 4 +--
.../qemuxml2argv-pci-bridge-many-disks.xml | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-pci-many.args | 8 +++---
tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml | 4 +--
.../qemuxml2xmlout-disk-source-pool.xml | 2 +-
.../qemuxml2xmlout-pci-bridge-many-disks.xml | 2 +-
15 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d95dd3e..1a51c1d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11676,6 +11676,35 @@ virDomainDiskControllerMatch(int controller_type, int disk_bus)
return false;
}
+/* Return true if there's a duplicate disk[]->dst name for the same bus */
+bool
+virDomainDiskDefDstDuplicates(virDomainDefPtr def)
+{
+ size_t i, j;
+
+ /* optimization */
+ if (def->ndisks <= 1)
+ return false;
+
+ for (i = 0; i < def->ndisks; i++) {
you could do "i = 1" here ...
+ for (j = 0; j < def->ndisks; j++) {
... and "j < i" to make it O(n!/2) :)
+ if (i == j)
+ continue; /* not ourselves */
And then you can drop this check.
+ if (def->disks[i]->bus == def->disks[j]->bus
&&
+ STREQ(def->disks[i]->dst, def->disks[j]->dst)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("target '%s' duplicated for disk sources
"
+ "'%s' and '%s'"),
+ def->disks[i]->dst,
This is kind of tricky. It is not the target that's causing the
problem, but rather the address we generate. I tried working it out
somehow, but my approach was not clean at all. This seems like a more
stable and understandable solution. Even though I'd like it to be
just temporary.
ACK with the above speed-up fixed.
9:45 p.m.
On 02/26/2015 04:59 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1142631
Patch 1 was found when adjusting the bz to get the disk by the pool source
instead of directly by file attribute. When using domblklist, the source
wasn't displayed. Followed the model of the iscsi '@name' and added
'@volume'
(eg, <source pool='default' volume='test-1g.img'/>)
Patch 2 started as a simple (hah!) double loop looking for duplicate dst
strings. I chose a separate API called before qemuCheckDiskConfig since
it's more of a "common" check that could be used by other drivers as
opposed
to the perhaps more specific single disk checks made. Besides I didn't want
a ndisk factorial checking algorithm each time a disk was checked for something
that could be checked once.
With the code in place, running make check tripped over a number of existing
tests that duplicated that '<target dev=...' value, so I fixed those. Of
interest for a few was that the test would duplicate the supposedly unique
alias id - those are cleaned up as well as a byproduct of the changes.
John Ferlan (2):
virsh: Adjust domblklist to understand disk volume syntax
disk: Disallow duplicated target 'dev' values
src/conf/domain_conf.c | 29 ++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 3 +++
.../qemuxml2argv-disk-scsi-disk-split.xml | 2 +-
...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml | 2 +-
.../qemuxml2argv-disk-scsi-lun-passthrough.xml | 2 +-
.../qemuxml2argv-disk-source-pool-mode.xml | 2 +-
.../qemuxml2argv-disk-source-pool.xml | 2 +-
.../qemuxml2argv-pci-bridge-many-disks.args | 4 +--
.../qemuxml2argv-pci-bridge-many-disks.xml | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-pci-many.args | 8 +++---
tests/qemuxml2argvdata/qemuxml2argv-pci-many.xml | 4 +--
.../qemuxml2xmlout-disk-source-pool.xml | 2 +-
.../qemuxml2xmlout-pci-bridge-many-disks.xml | 2 +-
tools/virsh-domain-monitor.c | 3 ++-
16 files changed, 52 insertions(+), 17 deletions(-)
Made the adjustments suggested and pushed - thanks for the review.
John
3552
days inactive
3557
days old
5 comments
2 participants
participants (2)
-
John Ferlan -
Martin Kletzander