12:40 p.m.
This patch series adds the functionality to detect a misconfiguration
between disk bus type and disk address type for disks that are using
the address type virDomainDeviceDriveAddress. It also adds a test for
it.
A check for other bus types may be needed. This may require a driver
specific function, as it is already implemented in
virDomainDeviceDefPostParse(), for example.
Marc Hartmayer (2):
conf: Detect misconfiguration between disk bus and disk address
tests: Add tests for disk configuration validation
src/conf/domain_conf.c | 46 ++++++++++++++++++++++
.../qemuxml2argv-disk-fdc-incompatible-address.xml | 22 +++++++++++
.../qemuxml2argv-disk-ide-incompatible-address.xml | 23 +++++++++++
...qemuxml2argv-disk-sata-incompatible-address.xml | 23 +++++++++++
...qemuxml2argv-disk-scsi-incompatible-address.xml | 24 +++++++++++
tests/qemuxml2argvtest.c | 8 ++++
6 files changed, 146 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-fdc-incompatible-address.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-ide-incompatible-address.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-sata-incompatible-address.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-incompatible-address.xml
--
2.5.5
12:40 p.m.
New subject: [libvirt] [PATCH 1/2] conf: Detect misconfiguration between disk bus and disk address
This patch detects a misconfiguration between the disk bus type and disk
address type for controller based disk buses (SATA, SCSI, FDC and
IDE). The addresses of these bus types are all managed in common code so
it's possible to decide in common code whether the disk address and bus
type are compatible or not.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6e008e2..0cdcc9f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4664,6 +4664,38 @@ virDomainDefPostParse(virDomainDefPtr def,
}
+/**
+ * virDomainDiskAddressDiskBusCompatibility:
+ * @bus: disk bus type
+ * @addressType: disk address type
+ *
+ * Check if the specified disk address type @addressType is compatible
+ * with the specified disk bus type @bus. This function checks
+ * compatibility with the bus types SATA, SCSI, FDC, and IDE only,
+ * because only these are handled in common code.
+ *
+ * Returns true if compatible or can't be decided in common code,
+ * false if known to be not compatible.
+ */
+static bool
+virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus,
+ virDomainDeviceAddressType addressType)
+{
+ if (addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+ return true;
+
+ switch (bus) {
+ case VIR_DOMAIN_DISK_BUS_SATA:
+ case VIR_DOMAIN_DISK_BUS_SCSI:
+ case VIR_DOMAIN_DISK_BUS_FDC:
+ case VIR_DOMAIN_DISK_BUS_IDE:
+ return addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
+ default:
+ return true;
+ }
+}
+
+
static int
virDomainDiskDefValidate(const virDomainDiskDef *disk)
{
@@ -4681,6 +4713,20 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
}
}
+ /* Reject disks with a bus type that is not compatible with the
+ * given address type. The function considers only buses that are
+ * handled in common code. For other bus types it's not possible
+ * to decide compatibility in common code.
+ */
+ if (!virDomainDiskAddressDiskBusCompatibility(disk->bus, disk->info.type)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid address type '%s' for the disk
'%s' with the bus type '%s'"),
+ virDomainDeviceAddressTypeToString(disk->info.type),
+ disk->dst,
+ virDomainDiskBusTypeToString(disk->bus));
+ return -1;
+ }
+
return 0;
}
--
2.5.5
2:09 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Detect misconfiguration between disk bus and disk address
On Tue, Nov 15, 2016 at 19:40:58 +0100, Marc Hartmayer wrote:
This patch detects a misconfiguration between the disk bus type and
disk
address type for controller based disk buses (SATA, SCSI, FDC and
IDE). The addresses of these bus types are all managed in common code so
it's possible to decide in common code whether the disk address and bus
type are compatible or not.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6e008e2..0cdcc9f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4664,6 +4664,38 @@ virDomainDefPostParse(virDomainDefPtr def,
}
+/**
+ * virDomainDiskAddressDiskBusCompatibility:
+ * @bus: disk bus type
+ * @addressType: disk address type
+ *
+ * Check if the specified disk address type @addressType is compatible
+ * with the specified disk bus type @bus. This function checks
+ * compatibility with the bus types SATA, SCSI, FDC, and IDE only,
+ * because only these are handled in common code.
+ *
+ * Returns true if compatible or can't be decided in common code,
+ * false if known to be not compatible.
+ */
+static bool
+virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus,
+ virDomainDeviceAddressType addressType)
+{
+ if (addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+ return true;
+
+ switch (bus) {
+ case VIR_DOMAIN_DISK_BUS_SATA:
+ case VIR_DOMAIN_DISK_BUS_SCSI:
+ case VIR_DOMAIN_DISK_BUS_FDC:
+ case VIR_DOMAIN_DISK_BUS_IDE:
+ return addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
+ default:
We tend to use a full enumeration of the types rather than the default
case along with a typecast of the switched variable to the correct type
so that the compiler checks if a new enum value is added.
+ return true;
+ }
+}
+
+
static int
virDomainDiskDefValidate(const virDomainDiskDef *disk)
{
@@ -4681,6 +4713,20 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
}
}
+ /* Reject disks with a bus type that is not compatible with the
+ * given address type. The function considers only buses that are
+ * handled in common code. For other bus types it's not possible
+ * to decide compatibility in common code.
+ */
This comment is kind of redundant with the comment of the function.
+ if (!virDomainDiskAddressDiskBusCompatibility(disk->bus,
disk->info.type)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid address type '%s' for the disk
'%s' with the bus type '%s'"),
+ virDomainDeviceAddressTypeToString(disk->info.type),
+ disk->dst,
+ virDomainDiskBusTypeToString(disk->bus));
+ return -1;
+ }
+
return 0;
}
Peter
4:21 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Detect misconfiguration between disk bus and disk address
On Wed, Nov 16, 2016 at 09:09 AM +0100, Peter Krempa <pkrempa(a)redhat.com> wrote:
> +{
> + if (addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> + return true;
> +
> + switch (bus) {
> + case VIR_DOMAIN_DISK_BUS_SATA:
> + case VIR_DOMAIN_DISK_BUS_SCSI:
> + case VIR_DOMAIN_DISK_BUS_FDC:
> + case VIR_DOMAIN_DISK_BUS_IDE:
> + return addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
> + default:
We tend to use a full enumeration of the types rather than the default
case along with a typecast of the switched variable to the correct type
so that the compiler checks if a new enum value is added.
Okay, I will change it.
Just a small question (maybe I should create another thread for it):
What's the reason why we're using 'int' and not directly the enum for
e.g. the field 'bus' in the 'struct t _virDomainDiskDef'?
<code>
...
int bus; /* enum virDomainDiskBus */
...
</code>
> static int
> virDomainDiskDefValidate(const virDomainDiskDef *disk)
> {
> @@ -4681,6 +4713,20 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
> }
> }
>
> + /* Reject disks with a bus type that is not compatible with the
> + * given address type. The function considers only buses that are
> + * handled in common code. For other bus types it's not possible
> + * to decide compatibility in common code.
> + */
This comment is kind of redundant with the comment of the function.
Yep - but I've added it for clarity because someone could think (if he
looks only at the function name) the function decides the compatibility for
EVERY bus type.
But if you want to I will remove it.
Anyway, thanks for comment.
Marc
12:40 p.m.
New subject: [libvirt] [PATCH 2/2] tests: Add tests for disk configuration validation
Add tests for controller based disks to check disk address compatibility
with disk bus types.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
---
.../qemuxml2argv-disk-fdc-incompatible-address.xml | 22 ++++++++++++++++++++
.../qemuxml2argv-disk-ide-incompatible-address.xml | 23 +++++++++++++++++++++
...qemuxml2argv-disk-sata-incompatible-address.xml | 23 +++++++++++++++++++++
...qemuxml2argv-disk-scsi-incompatible-address.xml | 24 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 8 ++++++++
5 files changed, 100 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-fdc-incompatible-address.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-ide-incompatible-address.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-sata-incompatible-address.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-incompatible-address.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-fdc-incompatible-address.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-fdc-incompatible-address.xml
new file mode 100644
index 0000000..f4aa6d5
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-fdc-incompatible-address.xml
@@ -0,0 +1,22 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='fd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='floppy'>
+ <source dev='/dev/fd0'/>
+ <target dev='fda' bus='fdc'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </disk>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-incompatible-address.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-incompatible-address.xml
new file mode 100644
index 0000000..9c2c887
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-incompatible-address.xml
@@ -0,0 +1,23 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </disk>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-sata-incompatible-address.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-sata-incompatible-address.xml
new file mode 100644
index 0000000..239e12b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-sata-incompatible-address.xml
@@ -0,0 +1,23 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='sata'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </disk>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-incompatible-address.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-incompatible-address.xml
new file mode 100644
index 0000000..45b41da
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-incompatible-address.xml
@@ -0,0 +1,24 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='scsi'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </disk>
+ <controller type='scsi' index='0'
model='virtio-scsi'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 985f45d..2dbd99a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -968,6 +968,14 @@ mymain(void)
DO_TEST("disk-serial",
QEMU_CAPS_KVM,
QEMU_CAPS_DRIVE_SERIAL);
+ DO_TEST_PARSE_ERROR("disk-fdc-incompatible-address",
+ NONE);
+ DO_TEST_PARSE_ERROR("disk-ide-incompatible-address",
+ NONE);
+ DO_TEST_PARSE_ERROR("disk-sata-incompatible-address",
+ QEMU_CAPS_ICH9_AHCI);
+ DO_TEST_PARSE_ERROR("disk-scsi-incompatible-address",
+ QEMU_CAPS_VIRTIO_SCSI);
DO_TEST("graphics-vnc", QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_CIRRUS_VGA);
DO_TEST("graphics-vnc-socket", QEMU_CAPS_VNC,
QEMU_CAPS_DEVICE_CIRRUS_VGA);
--
2.5.5
2959
days inactive
2960
days old
4 comments
2 participants
participants (2)
-
Marc Hartmayer -
Peter Krempa