[libvirt] [PATCH] Perform disk config validity checking for attach-device config
John Ferlan
jferlan at redhat.com
Mon Aug 4 15:04:41 UTC 2014
https://bugzilla.redhat.com/show_bug.cgi?id=1078126
Using 'virsh attach-device --config' (or --persistent) to attach a
file backed lun device will succeed; however, subsequent domain restarts
will result in failure because the configuration of a file backed lun
is not supported.
Although allowing 'illegal configurations' is something that can be
allowed, it may not be practical in this case. Generally, when attaching
a device to a domain means the domain must be running. A way around
this is using the --config (or --persistent) option. When an attach
is done to a running domain, a temporary configuration is modified
first followed by the live update. The live update will make a number
of disk validity checks when building the qemu command to attach the
disk. If any fail, then change is rejected.
Rather than allow a potentially illegal combination, adjust the code
in the configuration path to make the same checks as the running path
will make with respect to disk validity checks. This way we avoid
having the potential for some subsequent start/reboot to fail because
an illegal combination was allowed.
NB: The live checking remains since it is possible to just to --live
configuration modification.
Signed-off-by: John Ferlan <jferlan at redhat.com>
---
src/qemu/qemu_command.c | 138 ++++++++++++++++++++++++++----------------------
src/qemu/qemu_command.h | 3 ++
src/qemu/qemu_driver.c | 2 +
3 files changed, 81 insertions(+), 62 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4e074e5..8e56151 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3234,6 +3234,81 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
}
+int
+qemuBuildCheckDisk(virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps)
+{
+ if (virDiskNameToIndex(disk->dst) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unsupported disk type '%s'"), disk->dst);
+ goto error;
+ }
+
+ if (disk->wwn) {
+ if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) &&
+ (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Only ide and scsi disk support wwn"));
+ goto error;
+ }
+ }
+
+ if ((disk->vendor || disk->product) &&
+ disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Only scsi disk supports vendor and product"));
+ goto error;
+ }
+
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
+ /* make sure that both the bus and the qemu binary support
+ * type='lun' (SG_IO).
+ */
+ if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO &&
+ disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("disk device='lun' is not supported for bus='%s'"),
+ virDomainDiskQEMUBusTypeToString(disk->bus));
+ goto error;
+ }
+ if (disk->src->type == VIR_STORAGE_TYPE_NETWORK) {
+ if (disk->src->protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("disk device='lun' is not supported "
+ "for protocol='%s'"),
+ virStorageNetProtocolTypeToString(disk->src->protocol));
+ goto error;
+ }
+ } else if (!virDomainDiskSourceIsBlockType(disk)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disk device='lun' is only valid for block "
+ "type disk source"));
+ goto error;
+ }
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disk device='lun' is not supported by "
+ "this QEMU"));
+ goto error;
+ }
+ if (disk->wwn) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Setting wwn is not supported for lun device"));
+ goto error;
+ }
+ if (disk->vendor || disk->product) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Setting vendor or product is not supported "
+ "for lun device"));
+ goto error;
+ }
+ }
+ return 0;
+ error:
+ return -1;
+}
+
+
char *
qemuBuildDriveStr(virConnectPtr conn,
virDomainDiskDefPtr disk,
@@ -3620,71 +3695,10 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
{
virBuffer opt = VIR_BUFFER_INITIALIZER;
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
- int idx = virDiskNameToIndex(disk->dst);
int controllerModel;
- if (idx < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unsupported disk type '%s'"), disk->dst);
+ if (qemuBuildCheckDisk(disk, qemuCaps) < 0)
goto error;
- }
-
- if (disk->wwn) {
- if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) &&
- (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Only ide and scsi disk support wwn"));
- goto error;
- }
- }
-
- if ((disk->vendor || disk->product) &&
- disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Only scsi disk supports vendor and product"));
- goto error;
- }
-
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
- /* make sure that both the bus and the qemu binary support
- * type='lun' (SG_IO).
- */
- if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO &&
- disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("disk device='lun' is not supported for bus='%s'"),
- bus);
- goto error;
- }
- if (disk->src->type == VIR_STORAGE_TYPE_NETWORK) {
- if (disk->src->protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("disk device='lun' is not supported for protocol='%s'"),
- virStorageNetProtocolTypeToString(disk->src->protocol));
- goto error;
- }
- } else if (!virDomainDiskSourceIsBlockType(disk)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("disk device='lun' is only valid for block type disk source"));
- goto error;
- }
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("disk device='lun' is not supported by this QEMU"));
- goto error;
- }
- if (disk->wwn) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Setting wwn is not supported for lun device"));
- goto error;
- }
- if (disk->vendor || disk->product) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Setting vendor or product is not supported "
- "for lun device"));
- goto error;
- }
- }
switch (disk->bus) {
case VIR_DOMAIN_DISK_BUS_IDE:
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index b71e964..00097f9 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -271,4 +271,7 @@ qemuParseKeywords(const char *str,
int qemuGetDriveSourceString(virStorageSourcePtr src,
virConnectPtr conn,
char **source);
+
+int qemuBuildCheckDisk(virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps);
#endif /* __QEMU_COMMAND_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a3de784..5bc2785 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6744,6 +6744,8 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
_("target %s already exists"), disk->dst);
return -1;
}
+ if (qemuBuildCheckDisk(disk, qemuCaps) < 0)
+ return -1;
if (virDomainDiskInsert(vmdef, disk))
return -1;
/* vmdef has the pointer. Generic codes for vmdef will do all jobs */
--
1.9.3
More information about the libvir-list
mailing list