[libvirt] [PATCH 8/8] Revert "conf: Validate disk lun using correct types"

[John Ferlan]

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,
>