[libvirt] [PATCH 3/6] vz: check supported disk format and bus

[Maxim Nestratov]

15.03.2016 10:47, Mikhail Feoktistov пишет:
> In DomainPostParse phase we check disk format and bus.
> If disk format is not specified in XML (VIR_STORAGE_FILE_NONE) than we accept this disk.
> Otherwise we try to find disk format in vzCapabilities struct.
> Also we try to find disk bus in vzCapabilities->diskBuses.
> It contains buses that we support.
> ---
>   src/vz/vz_driver.c |  6 +++++-
>   src/vz/vz_sdk.c    | 24 ++----------------------
>   src/vz/vz_utils.c  | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/vz/vz_utils.h  |  3 +++
>   4 files changed, 62 insertions(+), 23 deletions(-)
>
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index 5d48d7e..4f52bc7 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -175,8 +175,11 @@ static int
>   vzDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED,
>                        virCapsPtr caps ATTRIBUTE_UNUSED,
>                        unsigned int parseFlags ATTRIBUTE_UNUSED,
> -                     void *opaque ATTRIBUTE_UNUSED)
> +                     void *opaque)
>   {
> +    if (vzCheckUnsupportedDisks(def, opaque) < 0)
> +        return -1;
> +
>       return 0;
>   }
>   
> @@ -239,6 +242,7 @@ vzOpenDefault(virConnectPtr conn)
>       if (!(privconn->caps = vzBuildCapabilities()))
>           goto error;
>   
> +    vzDomainDefParserConfig.priv = &privconn->vzCaps;
>       if (!(privconn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig,
>                                                      NULL, NULL)))
>           goto error;
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index de73c31..2a6e908 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -3143,30 +3143,10 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom,
>       pret = PrlVmDev_SetConnected(sdkdisk, 1);
>       prlsdkCheckRetGoto(pret, cleanup);
>   
> -    if (disk->src->type == VIR_STORAGE_TYPE_FILE) {
> -        if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
> -            virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_PLOOP) {
> -
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid format of "
> -                                                         "disk %s, vz driver supports only "
> -                                                         "images in ploop format."), disk->src->path);
> -            goto cleanup;
> -        }
> -
> +    if (disk->src->type == VIR_STORAGE_TYPE_FILE)
>           emutype = PDT_USE_IMAGE_FILE;
> -    } else {
> -        if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
> -            (virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_RAW &&
> -             virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_NONE &&
> -             virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_AUTO)) {
> -
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid format "
> -                                                         "of disk %s, it should be either not set, or set "
> -                                                         "to raw or auto."), disk->src->path);
> -            goto cleanup;
> -        }
> +    else
>           emutype = PDT_USE_REAL_DEVICE;

I know you keep the logic we currently have but this is not true because 
"not file" doesn't mean "real device"
and else if (disk->src->type == VIR_STORAGE_TYPE_BLOCK) would make sence 
here.

> -    }
>   
>       pret = PrlVmDev_SetEmulatedType(sdkdisk, emutype);
>       prlsdkCheckRetGoto(pret, cleanup);
> diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
> index d8a95ac..bf62538 100644
> --- a/src/vz/vz_utils.c
> +++ b/src/vz/vz_utils.c
> @@ -249,3 +249,55 @@ vzInitVersion(vzConnPtr privconn)
>       VIR_FREE(output);
>       return ret;
>   }
> +
> +int vzCheckUnsupportedDisks(virDomainDefPtr def,
> +                            vzCapabilitiesPtr vzCaps)
> +{
> +    size_t i, j;
> +    virDomainDiskDefPtr disk;
> +    virStorageFileFormat diskFormat;
> +    bool supported;
> +
> +    for (i = 0; i < def->ndisks; i++) {
> +        disk = def->disks[i];
> +        diskFormat = virDomainDiskGetFormat(disk);
> +        // disk format

unnecessary commentary

> +        supported = true;
> +        if (disk->src->type == VIR_STORAGE_TYPE_FILE) {
> +            if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
> +                diskFormat != VIR_STORAGE_FILE_NONE) {
> +
> +                if (IS_CT(def))
> +                    supported = vzCaps->ctDiskFormat == diskFormat;
> +                else
> +                    supported = vzCaps->vmDiskFormat == diskFormat;
> +            }
> +        } else {

again, else if (disk->src->type == VIR_STORAGE_TYPE_BLOCK) would make 
sence here

> +            if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> +                supported = diskFormat == VIR_STORAGE_FILE_RAW ||
> +                            diskFormat == VIR_STORAGE_FILE_NONE ||
> +                            diskFormat == VIR_STORAGE_FILE_AUTO;
> +            }
> +        }

else block with supported = false is required because there are 
VIR_STORAGE_TYPE_NETWORK
VIR_STORAGE_TYPE_DIR and VIR_STORAGE_TYPE_VOLUME

> +
> +        if (!supported) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Unsupported format of disk %s"),

swearing about unsupported format isn't completely correct here if we do 
proposed above correction

> +                           disk->src->path);
> +            return -1;
> +        }
> +        //disk bus
> +        for (j = 0; vzCaps->diskBuses[j] != VIR_DOMAIN_DISK_BUS_LAST; j++) {
> +            if (disk->bus == vzCaps->diskBuses[j])
> +                break;
> +        }
> +
> +        if (vzCaps->diskBuses[j] == VIR_DOMAIN_DISK_BUS_LAST) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Unsupported disk bus type %s"),
> +                           virDomainDiskBusTypeToString(disk->bus));
> +            return -1;
> +        }
> +    }
> +    return 0;
> +}
> diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
> index e0b0105..851322f 100644
> --- a/src/vz/vz_utils.h
> +++ b/src/vz/vz_utils.h
> @@ -110,6 +110,9 @@ vzNewDomain(vzConnPtr privconn,
>               const unsigned char *uuid);
>   int
>   vzInitVersion(vzConnPtr privconn);
> +int
> +vzCheckUnsupportedDisks(virDomainDefPtr def,
> +                        vzCapabilitiesPtr vzCaps);
>   
>   # define PARALLELS_BLOCK_STATS_FOREACH(OP)                              \
>       OP(rd_req, VIR_DOMAIN_BLOCK_STATS_READ_REQ, "read_requests")        \