[libvirt] [PATCH 0/6] RESEND vz: minor 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 | 29 +++++++++++++++++++---------- src/vz/vz_utils.c | 23 +++++++++-------------- 2 files changed, 28 insertions(+), 24 deletions(-) -- 2.4.3
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> 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 ae99aa0..49997f0 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -414,8 +414,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; } } -- 2.4.3
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> 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 49997f0..d9da1e7 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -392,7 +392,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]; @@ -400,30 +399,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; -- 2.4.3
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> Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b321d39..e373748 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3216,11 +3216,13 @@ static int prlsdkAddDisk(vzDriverPtr driver, pret = PrlVmDev_SetEmulatedType(sdkdisk, emutype); prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetSysName(sdkdisk, path); - prlsdkCheckRetGoto(pret, cleanup); + if (disk->src->path) { + pret = PrlVmDev_SetSysName(sdkdisk, path); + prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetFriendlyName(sdkdisk, path); - prlsdkCheckRetGoto(pret, cleanup); + pret = PrlVmDev_SetFriendlyName(sdkdisk, path); + prlsdkCheckRetGoto(pret, cleanup); + } drive = &disk->info.addr.drive; if (drive->controller > 0) { -- 2.4.3
17.05.2016 13:47, Maxim Nestratov пишет:
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> Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b321d39..e373748 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3216,11 +3216,13 @@ static int prlsdkAddDisk(vzDriverPtr driver, pret = PrlVmDev_SetEmulatedType(sdkdisk, emutype); prlsdkCheckRetGoto(pret, cleanup);
- pret = PrlVmDev_SetSysName(sdkdisk, path); - prlsdkCheckRetGoto(pret, cleanup); + if (disk->src->path) { + pret = PrlVmDev_SetSysName(sdkdisk, path); + prlsdkCheckRetGoto(pret, cleanup);
- pret = PrlVmDev_SetFriendlyName(sdkdisk, path); - prlsdkCheckRetGoto(pret, cleanup); + pret = PrlVmDev_SetFriendlyName(sdkdisk, path); + prlsdkCheckRetGoto(pret, cleanup); + }
drive = &disk->info.addr.drive; if (drive->controller > 0) {
Ooops. This should haven't been pushed. Reverted and pushed as trivial.
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> 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 e373748..e487f26 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2666,7 +2666,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; } -- 2.4.3
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 e487f26..b595fe7 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3435,6 +3435,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; @@ -3733,6 +3736,7 @@ prlsdkCreateCt(vzDriverPtr driver, 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; @@ -3777,8 +3781,10 @@ prlsdkCreateCt(vzDriverPtr driver, 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; -- 2.4.3
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Signed-off-by: Maxim Nestratov <mnestratov@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 b595fe7..4fe9bfa 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3777,8 +3777,7 @@ prlsdkCreateCt(vzDriverPtr driver, virDomainDefPtr def) } - ret = prlsdkDoApplyConfig(driver, sdkdom, def, NULL); - if (ret) + if (prlsdkDoApplyConfig(driver, sdkdom, def, NULL) < 0) goto cleanup; flags = PACF_NON_INTERACTIVE_MODE; @@ -3786,7 +3785,9 @@ prlsdkCreateCt(vzDriverPtr driver, 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); -- 2.4.3
17.05.2016 13:47, Maxim Nestratov пишет:
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 | 29 +++++++++++++++++++---------- src/vz/vz_utils.c | 23 +++++++++-------------- 2 files changed, 28 insertions(+), 24 deletions(-)
I experienced some glitches with my email and wasn't able to reply to original series https://www.redhat.com/archives/libvir-list/2016-March/msg01262.html So, I resent it here just to say that I ACKed it changed some nits after rebasing to current master and pushed. Thanks, Maxim
participants (1)
-
Maxim Nestratov