Re: [libvirt] [PATCH 8/8] Revert "conf: Validate disk lun using correct types"
On 05/02/2016 10:32 AM, Peter Krempa wrote:
This reverts commit c79ebf53b5fe0a33bf407b3bcb49e3a27ec97eb4.
We can't just add checks to the XML parser once we've accepted such
configuration in the past.
---
src/conf/domain_conf.c | 22 ----------------------
tests/qemuxml2argvtest.c | 3 +--
2 files changed, 1 insertion(+), 24 deletions(-)
There was a bz associated with that commit - that'll need to be
addressed in some manner...
While I understand your point here, the configuration didn't work - that
is it couldn't be started anyway so there could not be a domain running
with that configuration and thus it wouldn't disappear on a subsequent
reload, hence why checking the config and rejecting "earlier" seemed
proper even though we hadn't rejected such a config when the
"mode='host'" was first implemented.
Commit 'c79ebf53b' is a followup of sorts to commit '33188c9f'...
For me it's a NACK, but someone else may feel differently
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 48a220f..c4cbbff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4181,28 +4181,6 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
}
}
- /* Validate LUN configuration
- * NOTE: virStorageTranslateDiskSourcePool is not run yet, so for
- * disk "volume"'s, the closest we can get at config time
is
- * to ensure mode isn't direct since host/default will allow
- * lun/block usage. At run time if it's determined the wrong
- * voltype and pooltype values are set, then failure occurs
- */
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN &&
- !(disk->src->type == VIR_STORAGE_TYPE_BLOCK ||
- (disk->src->type == VIR_STORAGE_TYPE_NETWORK &&
- disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) ||
- (disk->src->type == VIR_STORAGE_TYPE_VOLUME &&
- disk->src->srcpool &&
- disk->src->srcpool->mode !=
- VIR_STORAGE_SOURCE_POOL_MODE_DIRECT))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("disk '%s' improperly configured for a "
- "device='lun'"),
- disk->dst);
- return -1;
- }
-
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0)
return -1;
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8842b2f..4211e82 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -780,8 +780,7 @@ mymain(void)
DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE);
DO_TEST("disk-drive-no-boot",
QEMU_CAPS_BOOTINDEX);
- DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid",
- QEMU_CAPS_VIRTIO_SCSI);
+ DO_TEST_FAILURE("disk-device-lun-type-invalid", QEMU_CAPS_VIRTIO_SCSI);
DO_TEST_FAILURE("disk-usb-nosupport", NONE);
DO_TEST("disk-usb-device",
QEMU_CAPS_DEVICE_USB_STORAGE,