[libvirt] [PATCH 0/6] vz: misc fixes

Mikhail Feoktistov (2): vz: handle sourceless cdroms vz: fix template ct creation Nikolay Shirokovskiy (4): vz: remove check for auto file format for disks vz: fix vzCheckUnsupportedDisks format checks for cdroms vz: fix error message for readonly fs vz: make error path code idiomatic src/vz/vz_sdk.c | 31 ++++++++++++++++++++----------- src/vz/vz_utils.c | 23 +++++++++-------------- 2 files changed, 29 insertions(+), 25 deletions(-) -- 1.8.3.1

VIR_STORAGE_FILE_AUTO can not be set from xml description. At the same time we don't set disks format to this value as for example qemu does. Thus this we can never get this value in format. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_utils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index fed48a5..e6b0a27 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -415,8 +415,7 @@ vzCheckUnsupportedDisks(virDomainDefPtr def, vzCapabilitiesPtr vzCaps) } else { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { supported = diskFormat == VIR_STORAGE_FILE_RAW || - diskFormat == VIR_STORAGE_FILE_NONE || - diskFormat == VIR_STORAGE_FILE_AUTO; + diskFormat == VIR_STORAGE_FILE_NONE; } } -- 1.8.3.1

Current version of the function does not check format of cdroms at all. At the same time prlsdkGetDiskInfo give hints that cdroms always have format VIR_STORAGE_FILE_RAW. So fix vzCheckUnsupportedDisks. About structure of checks. As we don't have means to store format in SDK we always have only one format in every situation. So instead of setting boolean let's get allowed format instead and finally compare it to the requested. This structure of checks seems stable to me because we have only one format in every situation. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_utils.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index e6b0a27..ed1756f 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -393,7 +393,6 @@ 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]; @@ -401,30 +400,27 @@ vzCheckUnsupportedDisks(virDomainDefPtr def, vzCapabilitiesPtr vzCaps) if (vzCheckDiskUnsupportedParams(disk) < 0) return -1; - diskFormat = virDomainDiskGetFormat(disk); - supported = true; if (disk->src->type == VIR_STORAGE_TYPE_FILE) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && - diskFormat != VIR_STORAGE_FILE_NONE) { - + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { if (IS_CT(def)) - supported = vzCaps->ctDiskFormat == diskFormat; + diskFormat = vzCaps->ctDiskFormat; else - supported = vzCaps->vmDiskFormat == diskFormat; + diskFormat = vzCaps->vmDiskFormat; + } else { + diskFormat = VIR_STORAGE_FILE_RAW; } } else { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - supported = diskFormat == VIR_STORAGE_FILE_RAW || - diskFormat == VIR_STORAGE_FILE_NONE; - } + diskFormat = VIR_STORAGE_FILE_RAW; } - if (!supported) { + if (virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_NONE && + virDomainDiskGetFormat(disk) != diskFormat) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported format of disk %s"), disk->src->path); return -1; } + for (j = 0; vzCaps->diskBuses[j] != VIR_DOMAIN_DISK_BUS_LAST; j++) { if (disk->bus == vzCaps->diskBuses[j]) break; -- 1.8.3.1

From: Mikhail Feoktistov <mfeoktistov@virtuozzo.com> SDK handles empty cdroms all right. We just need to pass "" instead of NULL (not setting is good too). However we can get problems here. Disk detaching treats source as ids. Fortunately disk detaching is not supported for cdroms yet and for hard disks we can not get empty source - this is prohibitited by xml parsing code. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_sdk.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 8691887..7fa9959 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -506,7 +506,7 @@ prlsdkGetDiskInfo(vzConnPtr privconn, pret = PrlVmDev_GetFriendlyName(prldisk, buf, &buflen); prlsdkCheckRetGoto(pret, cleanup); - if (virDomainDiskSetSource(disk, buf) < 0) + if (strlen(buf) != 0 && virDomainDiskSetSource(disk, buf) < 0) goto cleanup; /* Let physical devices added to CT look like SATA disks */ @@ -3029,11 +3029,13 @@ static int prlsdkAddDisk(vzConnPtr privconn, pret = PrlVmDev_SetEmulatedType(sdkdisk, emutype); prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetSysName(sdkdisk, disk->src->path); - prlsdkCheckRetGoto(pret, cleanup); + if (disk->src->path) { + pret = PrlVmDev_SetSysName(sdkdisk, disk->src->path); + prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetFriendlyName(sdkdisk, disk->src->path); - prlsdkCheckRetGoto(pret, cleanup); + pret = PrlVmDev_SetFriendlyName(sdkdisk, disk->src->path); + prlsdkCheckRetGoto(pret, cleanup); + } drive = &disk->info.addr.drive; if (drive->controller > 0) { -- 1.8.3.1

On 28.03.2016 11:00, Nikolay Shirokovskiy wrote:
From: Mikhail Feoktistov <mfeoktistov@virtuozzo.com>
SDK handles empty cdroms all right. We just need to pass "" instead of NULL (not setting is good too).
However we can get problems here. Disk detaching treats source as ids. Fortunately disk detaching is not supported for cdroms yet and for hard disks we can not get empty source - this is prohibitited by xml parsing code.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_sdk.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
Please drop this patch from the series. I sent a new version as a standalone patch. Nikolay

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_sdk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 7fa9959..b589f4e 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2455,7 +2455,7 @@ static int prlsdkCheckFSUnsupportedParams(virDomainFSDefPtr fs) if (fs->readonly) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Setting readonly for filesystems is " - "supported by vz driver.")); + "not supported by vz driver.")); return -1; } -- 1.8.3.1

From: Mikhail Feoktistov <mfeoktistov@virtuozzo.com> First we don't need to add disk in this case. Second flag should be skipped. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_sdk.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b589f4e..7937699 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3269,6 +3269,9 @@ prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs) PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE; int ret = -1; + if (fs->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) + return 0; + if (prlsdkCheckFSUnsupportedParams(fs) < 0) return -1; @@ -3504,6 +3507,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) PRL_HANDLE job = PRL_INVALID_HANDLE; PRL_HANDLE result = PRL_INVALID_HANDLE; PRL_RESULT pret; + PRL_UINT32 flags; int ret = -1; int useTemplate = 0; size_t i; @@ -3548,8 +3552,10 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) if (ret) goto cleanup; - job = PrlVm_RegEx(sdkdom, "", - PACF_NON_INTERACTIVE_MODE | PRNVM_PRESERVE_DISK); + flags = PACF_NON_INTERACTIVE_MODE; + if (!useTemplate) + flags |= PRNVM_PRESERVE_DISK; + job = PrlVm_RegEx(sdkdom, "", flags); if (PRL_FAILED(waitJob(job))) ret = -1; -- 1.8.3.1

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_sdk.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 7937699..cca5459 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3548,8 +3548,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) } - ret = prlsdkDoApplyConfig(conn, sdkdom, def, NULL); - if (ret) + if (prlsdkDoApplyConfig(conn, sdkdom, def, NULL) < 0) goto cleanup; flags = PACF_NON_INTERACTIVE_MODE; @@ -3557,7 +3556,9 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) flags |= PRNVM_PRESERVE_DISK; job = PrlVm_RegEx(sdkdom, "", flags); if (PRL_FAILED(waitJob(job))) - ret = -1; + goto cleanup; + + ret = 0; cleanup: PrlHandle_Free(sdkdom); -- 1.8.3.1
participants (1)
-
Nikolay Shirokovskiy