[libvirt] [PATCH v3 00/14] parallels: implement managed save

This patch series is intended to implement all needed code, so that suspend/resume in openstack nova will work. It implements .domainHasManagedSaveImage, .domainManagedSave and .domainManagedSaveRemove functions. It's not possible to change VM configuration, if it is in suspended state in PCS. It's possible for containers, but it has different meaning from the same action in libvirt: libvirt stores domain config in a save file and restores it if a user decided to start domain. But if the user discarded state file - domain will have new configuration. PCS doesn't store domain config for containers, so it will have new configuration either if the user will start it from, save file or discard state file and start after that. So the idea is to forbid changing domain configuration, if a domain is in managed saved state. We can't forbid calling this function in this state, because openstack/nova calls virDomainDefineXML in suspended state. So I compare current virDomainDef with the new one. And if there are no changes - function does nothing and returns success. Dmitry Guryanov (14): parallels: fix headers in parallels_sdk.h parallels: split prlsdkDomainChangeState function parallels: implement virDomainManagedSave parallels: report, that cdroms are readonly parallels: add controllers in prlsdkLoadDomain parallels: fill adapter model in virDomainNetDef parallels: don't fill net adapter model for containers conf: add VIR_DOMAIN_VIDEO_TYPE_PARALLELS video type conf: return proper default video type for parallels conf: add input device type for parallels containers parallels: add implicit input devices conf: fix virDomainDefParseXML for parallels conf: fix virDomainDefFormatInternal for parallels parallels: fix virDomainDefineXML for domain in saved state src/conf/domain_conf.c | 70 +++++++++++++++++++--- src/conf/domain_conf.h | 2 + src/parallels/parallels_driver.c | 104 ++++++++++++++++++++++++++++++-- src/parallels/parallels_sdk.c | 124 ++++++++++++++++++++++++++++++++++----- src/parallels/parallels_sdk.h | 17 ++++-- src/qemu/qemu_command.c | 6 +- 6 files changed, 285 insertions(+), 38 deletions(-) -- 2.1.0

Return value of functions prlsdkStart/Kill/Stop e.t.c. is PRL_RESULT in parallels_sdk.c and int in parallels_sdk.h. PRL_RESULT is int, so compiler didn't report errors. Let's fix the difference. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_sdk.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h index 694c19b..cb8d3fb 100644 --- a/src/parallels/parallels_sdk.h +++ b/src/parallels/parallels_sdk.h @@ -35,11 +35,11 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned char *uuid); int prlsdkUpdateDomain(parallelsConnPtr privconn, virDomainObjPtr dom); int prlsdkSubscribeToPCSEvents(parallelsConnPtr privconn); void prlsdkUnsubscribeFromPCSEvents(parallelsConnPtr privconn); -int prlsdkStart(parallelsConnPtr privconn, PRL_HANDLE sdkdom); -int prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE sdkdom); -int prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom); -int prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom); -int prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom); +PRL_RESULT prlsdkStart(parallelsConnPtr privconn, PRL_HANDLE sdkdom); +PRL_RESULT prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE sdkdom); +PRL_RESULT prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom); +PRL_RESULT prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom); +PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom); typedef PRL_RESULT (*prlsdkChangeStateFunc)(parallelsConnPtr privconn, PRL_HANDLE sdkdom); int -- 2.1.0

Split function prlsdkDomainChangeState into prlsdkDomainChangeStateLocked and prlsdkDomainChangeState. So it can be used from places, where virDomainObj already found and locked. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_sdk.c | 35 +++++++++++++++++++++-------------- src/parallels/parallels_sdk.h | 4 ++++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index c36b772..d985a93 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1721,22 +1721,14 @@ PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom) } int -prlsdkDomainChangeState(virDomainPtr domain, - prlsdkChangeStateFunc chstate) +prlsdkDomainChangeStateLocked(parallelsConnPtr privconn, + virDomainObjPtr dom, + prlsdkChangeStateFunc chstate) { - parallelsConnPtr privconn = domain->conn->privateData; - virDomainObjPtr dom; parallelsDomObjPtr pdom; PRL_RESULT pret; - int ret = -1; virErrorNumber virerr; - dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid); - if (dom == NULL) { - parallelsDomNotFoundError(domain); - return -1; - } - pdom = dom->privateData; pret = chstate(privconn, pdom->sdkdom); if (PRL_FAILED(pret)) { @@ -1752,12 +1744,27 @@ prlsdkDomainChangeState(virDomainPtr domain, } virReportError(virerr, "%s", _("Can't change domain state.")); - goto cleanup; + return -1; } - ret = prlsdkUpdateDomain(privconn, dom); + return prlsdkUpdateDomain(privconn, dom); +} - cleanup: +int +prlsdkDomainChangeState(virDomainPtr domain, + prlsdkChangeStateFunc chstate) +{ + parallelsConnPtr privconn = domain->conn->privateData; + virDomainObjPtr dom; + int ret = -1; + + dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid); + if (dom == NULL) { + parallelsDomNotFoundError(domain); + return -1; + } + + ret = prlsdkDomainChangeStateLocked(privconn, dom, chstate); virObjectUnlock(dom); return ret; } diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h index cb8d3fb..780a226 100644 --- a/src/parallels/parallels_sdk.h +++ b/src/parallels/parallels_sdk.h @@ -46,6 +46,10 @@ int prlsdkDomainChangeState(virDomainPtr domain, prlsdkChangeStateFunc chstate); int +prlsdkDomainChangeStateLocked(parallelsConnPtr privconn, + virDomainObjPtr dom, + prlsdkChangeStateFunc chstate); +int prlsdkApplyConfig(virConnectPtr conn, virDomainObjPtr dom, virDomainDefPtr new); -- 2.1.0

Implement virDomainManagedSave api function. In PCS this feature called "suspend". You can suspend VM or CT while it is in running or paused state. And after resuming (or starting) it will have the same state, as before suspend. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 69 +++++++++++++++++++++++++++++++++++++++- src/parallels/parallels_sdk.c | 21 ++++++++++++ src/parallels/parallels_sdk.h | 3 ++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index f5e58a8..3102007 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -990,6 +990,8 @@ parallelsDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) { parallelsConnPtr privconn = domain->conn->privateData; virDomainObjPtr dom = NULL; + int state, reason; + int ret = 0; virCheckFlags(0, -1); @@ -999,9 +1001,72 @@ parallelsDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) return -1; } + state = virDomainObjGetState(dom, &reason); + if (state == VIR_DOMAIN_SHUTOFF && reason == VIR_DOMAIN_SHUTOFF_SAVED) + ret = 1; virObjectUnlock(dom); - return 0; + return ret; +} + +static int +parallelsDomainManagedSave(virDomainPtr domain, unsigned int flags) +{ + parallelsConnPtr privconn = domain->conn->privateData; + virDomainObjPtr dom = NULL; + int state, reason; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_SAVE_RUNNING | + VIR_DOMAIN_SAVE_PAUSED, -1); + + dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid); + if (dom == NULL) { + parallelsDomNotFoundError(domain); + return -1; + } + + state = virDomainObjGetState(dom, &reason); + + if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED)) { + ret = prlsdkDomainChangeStateLocked(privconn, dom, prlsdkPause); + if (ret) + goto cleanup; + } + + ret = prlsdkDomainChangeStateLocked(privconn, dom, prlsdkSuspend); + + cleanup: + virObjectUnlock(dom); + return ret; +} + +static int +parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) +{ + parallelsConnPtr privconn = domain->conn->privateData; + virDomainObjPtr dom = NULL; + int state, reason; + int ret = -1; + + virCheckFlags(0, -1); + + dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid); + if (dom == NULL) { + parallelsDomNotFoundError(domain); + return -1; + } + + state = virDomainObjGetState(dom, &reason); + + if (!(state == VIR_DOMAIN_SHUTOFF && reason == VIR_DOMAIN_SHUTOFF_SAVED)) + goto cleanup; + + ret = prlsdkDomainManagedSaveRemove(privconn, dom); + + cleanup: + virObjectUnlock(dom); + return ret; } static virHypervisorDriver parallelsDriver = { @@ -1046,6 +1111,8 @@ static virHypervisorDriver parallelsDriver = { .connectIsSecure = parallelsConnectIsSecure, /* 1.2.5 */ .connectIsAlive = parallelsConnectIsAlive, /* 1.2.5 */ .domainHasManagedSaveImage = parallelsDomainHasManagedSaveImage, /* 1.2.13 */ + .domainManagedSave = parallelsDomainManagedSave, /* 1.2.14 */ + .domainManagedSaveRemove = parallelsDomainManagedSaveRemove, /* 1.2.14 */ }; static virConnectDriver parallelsConnectDriver = { diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d985a93..7a90eec 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1720,6 +1720,14 @@ PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom) return waitJob(job, privconn->jobTimeout); } +PRL_RESULT prlsdkSuspend(parallelsConnPtr privconn, PRL_HANDLE sdkdom) +{ + PRL_HANDLE job = PRL_INVALID_HANDLE; + + job = PrlVm_Suspend(sdkdom); + return waitJob(job, privconn->jobTimeout); +} + int prlsdkDomainChangeStateLocked(parallelsConnPtr privconn, virDomainObjPtr dom, @@ -3204,3 +3212,16 @@ prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom) virDomainObjListRemove(privconn->domains, dom); return 0; } + +int +prlsdkDomainManagedSaveRemove(parallelsConnPtr privconn, virDomainObjPtr dom) +{ + parallelsDomObjPtr privdom = dom->privateData; + PRL_HANDLE job; + + job = PrlVm_DropSuspendedState(privdom->sdkdom); + if (PRL_FAILED(waitJob(job, privconn->jobTimeout))) + return -1; + + return 0; +} diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h index 780a226..b084678 100644 --- a/src/parallels/parallels_sdk.h +++ b/src/parallels/parallels_sdk.h @@ -40,6 +40,7 @@ PRL_RESULT prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE sdkdom); PRL_RESULT prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom); PRL_RESULT prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom); PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom); +PRL_RESULT prlsdkSuspend(parallelsConnPtr privconn, PRL_HANDLE sdkdom); typedef PRL_RESULT (*prlsdkChangeStateFunc)(parallelsConnPtr privconn, PRL_HANDLE sdkdom); int @@ -57,3 +58,5 @@ int prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def); int prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def); int prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom); +int +prlsdkDomainManagedSaveRemove(parallelsConnPtr privconn, virDomainObjPtr dom); -- 2.1.0

Set readonly flag for cdrom devices when we retrieve a list of domains from PCS. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_sdk.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 7a90eec..a49f792 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -478,10 +478,12 @@ prlsdkGetDiskInfo(PRL_HANDLE prldisk, virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); } - if (isCdrom) + if (isCdrom) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; - else + disk->src->readonly = true; + } else { disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; + } pret = PrlVmDev_GetFriendlyName(prldisk, NULL, &buflen); prlsdkCheckRetGoto(pret, cleanup); -- 2.1.0

Call virDomainDefAddImplicitControllers to add disk controllers, so virDomainDef, filled by this function will look exactly like the one returned by virDomainDefParseString. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_sdk.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index a49f792..888a3eb 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1265,6 +1265,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn, *s = '\0'; } + if (virDomainDefAddImplicitControllers(def) < 0) + goto error; + if (olddom) { /* assign new virDomainDef without any checks */ /* we can't use virDomainObjAssignDef, because it checks -- 2.1.0

We handle this parameter for VMs while defining domains, so let's get this property from PCS and set corresponding field of virDomainNetDef in prlsdkLoadDomains function. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_sdk.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 888a3eb..50b8a72 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -754,6 +754,31 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) } + if (!isCt) { + PRL_VM_NET_ADAPTER_TYPE type; + pret = PrlVmDevNet_GetAdapterType(netAdapter, &type); + prlsdkCheckRetGoto(pret, cleanup); + + switch (type) { + case PNT_RTL: + if (VIR_STRDUP(net->model, "rtl8139") < 0) + goto cleanup; + break; + case PNT_E1000: + if (VIR_STRDUP(net->model, "e1000") < 0) + goto cleanup; + break; + case PNT_VIRTIO: + if (VIR_STRDUP(net->model, "virtio") < 0) + goto cleanup; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown adapter type: %X"), type); + goto cleanup; + } + } + pret = PrlVmDev_IsConnected(netAdapter, &isConnected); prlsdkCheckRetGoto(pret, cleanup); -- 2.1.0

Network adapter model has no sense for container, so we shouldn't set it to e1000 in parallelsDomainDeviceDefPostParse. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 3102007..c3285db 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -174,7 +174,7 @@ parallelsDomainDefPostParse(virDomainDefPtr def, static int parallelsDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, - const virDomainDef *def ATTRIBUTE_UNUSED, + const virDomainDef *def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -184,6 +184,7 @@ parallelsDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, (dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK || dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && !dev->data.net->model && + STREQ(def->os.type, "hvm") && VIR_STRDUP(dev->data.net->model, "e1000") < 0) goto cleanup; -- 2.1.0

We support VNC for containers to have the same interface with VMs. At this moment it just renders linux text console. Of course we don't pass any physical devices and don't emulate virtual devices. Our VNC server renders text from terminal master and sends input events from VNC client to terminal. So add special video type VIR_DOMAIN_VIDEO_TYPE_PARALLELS for these pseudo-devices. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/parallels/parallels_sdk.c | 19 +++++++++++++++++++ src/qemu/qemu_command.c | 6 ++++-- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1763305..0ae45e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -500,7 +500,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vmvga", "xen", "vbox", - "qxl") + "qxl", + "parallels") VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST, "mouse", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0b2f1c9..ab11c0b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1326,6 +1326,7 @@ typedef enum { VIR_DOMAIN_VIDEO_TYPE_XEN, VIR_DOMAIN_VIDEO_TYPE_VBOX, VIR_DOMAIN_VIDEO_TYPE_QXL, + VIR_DOMAIN_VIDEO_TYPE_PARALLELS, /* pseudo device for VNC in containers */ VIR_DOMAIN_VIDEO_TYPE_LAST } virDomainVideoType; diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 50b8a72..8116691 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -997,6 +997,25 @@ prlsdkAddVNCInfo(PRL_HANDLE sdkdom, virDomainDefPtr def) if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, gr) < 0) goto error; + if (IS_CT(def)) { + virDomainVideoDefPtr video; + if (VIR_ALLOC(video) < 0) + goto error; + video->type = virDomainVideoDefaultType(def); + if (video->type < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot determine default video type")); + VIR_FREE(video); + goto error; + } + video->vram = virDomainVideoDefaultRAM(def, video->type); + video->heads = 1; + if (VIR_ALLOC_N(def->videos, 1) < 0) { + virDomainVideoDefFree(video); + goto error; + } + def->videos[def->nvideos++] = video; + } return 0; error: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3f0df58..5103599 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -112,7 +112,8 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vmware", "", /* no arg needed for xen */ "", /* don't support vbox */ - "qxl"); + "qxl", + "" /* don't support parallels */); VIR_ENUM_DECL(qemuDeviceVideo) @@ -122,7 +123,8 @@ VIR_ENUM_IMPL(qemuDeviceVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vmware-svga", "", /* no device for xen */ "", /* don't support vbox */ - "qxl-vga"); + "qxl-vga", + "" /* don't support parallels */); VIR_ENUM_DECL(qemuSoundCodec) -- 2.1.0

Fix function virDomainVideoDefaultType for parallels VMs and containers. It should return VGA for VMs and VIR_DOMAIN_VIDEO_TYPE_PARALLELS for containers. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/conf/domain_conf.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0ae45e1..c38a53c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10968,6 +10968,16 @@ virDomainVideoDefaultType(const virDomainDef *def) case VIR_DOMAIN_VIRT_VMWARE: return VIR_DOMAIN_VIDEO_TYPE_VMVGA; + case VIR_DOMAIN_VIRT_PARALLELS: + if (def->os.type) { + if (STREQ(def->os.type, "hvm")) + return VIR_DOMAIN_VIDEO_TYPE_VGA; + else + return VIR_DOMAIN_VIDEO_TYPE_PARALLELS; + } else { + return VIR_DOMAIN_VIDEO_TYPE_VGA; + } + default: return -1; } -- 2.1.0

Add VIR_DOMAIN_INPUT_BUS_PARALLELS device type to handle domain configuration properly for parallels containers, when VNC is enabled. When domain configuration has at least one 'graphics', there should be mouse and keyboard. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c38a53c..fbf8052 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -511,7 +511,8 @@ VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST, VIR_ENUM_IMPL(virDomainInputBus, VIR_DOMAIN_INPUT_BUS_LAST, "ps2", "usb", - "xen") + "xen", + "parallels") VIR_ENUM_IMPL(virDomainGraphics, VIR_DOMAIN_GRAPHICS_TYPE_LAST, "sdl", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ab11c0b..61082c8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1252,6 +1252,7 @@ typedef enum { VIR_DOMAIN_INPUT_BUS_PS2, VIR_DOMAIN_INPUT_BUS_USB, VIR_DOMAIN_INPUT_BUS_XEN, + VIR_DOMAIN_INPUT_BUS_PARALLELS, /* pseudo device for VNC in containers */ VIR_DOMAIN_INPUT_BUS_LAST } virDomainInputBus; -- 2.1.0

Add implicit input devices in parallelsLoadDomains, when VNC is enabled. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_sdk.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 8116691..f4602c8 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1312,6 +1312,21 @@ prlsdkLoadDomain(parallelsConnPtr privconn, if (virDomainDefAddImplicitControllers(def) < 0) goto error; + if (def->ngraphics > 0) { + int bus = IS_CT(def) ? VIR_DOMAIN_INPUT_BUS_PARALLELS: + VIR_DOMAIN_INPUT_BUS_PS2; + + if (virDomainDefMaybeAddInput(def, + VIR_DOMAIN_INPUT_TYPE_MOUSE, + bus) < 0) + goto error; + + if (virDomainDefMaybeAddInput(def, + VIR_DOMAIN_INPUT_TYPE_KBD, + bus) < 0) + goto error; + } + if (olddom) { /* assign new virDomainDef without any checks */ /* we can't use virDomainObjAssignDef, because it checks -- 2.1.0

Handle input devices in virDomainDefParseXML properly in case of parallels containers and VMs. Parallels containers support only VIR_DOMAIN_INPUT_BUS_PARALLELS. And if VNC is enabled we should add implicit mouse and keyboard. For VMs we should add implicit PS/2 mouse and keyboard. BTW, is it worth to refactor code and move all this code to drivers, to *DomainDefPostParse functions? Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/conf/domain_conf.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fbf8052..b8a6b84 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9458,7 +9458,7 @@ virDomainInputDefParseXML(const virDomainDef *dom, bus); goto error; } - } else { + } else if (STREQ(dom->os.type, "xen")) { if (def->bus != VIR_DOMAIN_INPUT_BUS_XEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported input bus %s"), @@ -9472,6 +9472,29 @@ virDomainInputDefParseXML(const virDomainDef *dom, type); goto error; } + } else { + if (dom->virtType == VIR_DOMAIN_VIRT_PARALLELS) { + if (def->bus != VIR_DOMAIN_INPUT_BUS_PARALLELS) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("parallels containers don't support " + "input bus %s"), + bus); + goto error; + } + + if (def->type != VIR_DOMAIN_INPUT_TYPE_MOUSE && + def->type != VIR_DOMAIN_INPUT_TYPE_KBD) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("parallels bus does not support " + "%s input device"), + type); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Input devices are not supported by this " + "virtualization driver.")); + } } } else { if (STREQ(dom->os.type, "hvm")) { @@ -9482,8 +9505,11 @@ virDomainInputDefParseXML(const virDomainDef *dom, } else { def->bus = VIR_DOMAIN_INPUT_BUS_USB; } - } else { + } else if (STREQ(dom->os.type, "xen")) { def->bus = VIR_DOMAIN_INPUT_BUS_XEN; + } else { + if ((dom->virtType == VIR_DOMAIN_VIRT_PARALLELS)) + def->bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; } } @@ -14979,9 +15005,14 @@ virDomainDefParseXML(xmlDocPtr xml, input->bus == VIR_DOMAIN_INPUT_BUS_PS2 && (input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || input->type == VIR_DOMAIN_INPUT_TYPE_KBD)) || - (STRNEQ(def->os.type, "hvm") && + (STREQ(def->os.type, "xen") && input->bus == VIR_DOMAIN_INPUT_BUS_XEN && (input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || + input->type == VIR_DOMAIN_INPUT_TYPE_KBD)) || + (STREQ(def->os.type, "exe") && + def->virtType == VIR_DOMAIN_VIRT_PARALLELS && + input->bus == VIR_DOMAIN_INPUT_BUS_PARALLELS && + (input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || input->type == VIR_DOMAIN_INPUT_TYPE_KBD))) { virDomainInputDefFree(input); continue; @@ -15014,6 +15045,9 @@ virDomainDefParseXML(xmlDocPtr xml, if (STREQ(def->os.type, "hvm")) input_bus = VIR_DOMAIN_INPUT_BUS_PS2; + if (STREQ(def->os.type, "exe") && + def->virtType == VIR_DOMAIN_VIRT_PARALLELS) + input_bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; if (virDomainDefMaybeAddInput(def, VIR_DOMAIN_INPUT_TYPE_MOUSE, -- 2.1.0

On 07.04.2015 22:35, Dmitry Guryanov wrote:
Handle input devices in virDomainDefParseXML properly in case of parallels containers and VMs.
Parallels containers support only VIR_DOMAIN_INPUT_BUS_PARALLELS. And if VNC is enabled we should add implicit mouse and keyboard.
For VMs we should add implicit PS/2 mouse and keyboard.
BTW, is it worth to refactor code and move all this code to drivers, to *DomainDefPostParse functions?
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/conf/domain_conf.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fbf8052..b8a6b84 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9458,7 +9458,7 @@ virDomainInputDefParseXML(const virDomainDef *dom, bus); goto error; } - } else { + } else if (STREQ(dom->os.type, "xen")) { if (def->bus != VIR_DOMAIN_INPUT_BUS_XEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported input bus %s"), @@ -9472,6 +9472,29 @@ virDomainInputDefParseXML(const virDomainDef *dom, type); goto error; } + } else { + if (dom->virtType == VIR_DOMAIN_VIRT_PARALLELS) { + if (def->bus != VIR_DOMAIN_INPUT_BUS_PARALLELS) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("parallels containers don't support " + "input bus %s"), + bus); + goto error; + } + + if (def->type != VIR_DOMAIN_INPUT_TYPE_MOUSE && + def->type != VIR_DOMAIN_INPUT_TYPE_KBD) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("parallels bus does not support " + "%s input device"), + type); + goto error; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Input devices are not supported by this " + "virtualization driver."));
Missing 'goto error;' Michal

We should add input devices with proper bus, not VIR_DOMAIN_INPUT_BUS_XEN. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/conf/domain_conf.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8a6b84..da81e72 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21176,12 +21176,18 @@ virDomainDefFormatInternal(virDomainDefPtr def, /* If graphics is enabled, add the implicit mouse/keyboard */ if ((ARCH_IS_X86(def->os.arch)) || def->os.arch == VIR_ARCH_NONE) { virDomainInputDef autoInput = { - VIR_DOMAIN_INPUT_TYPE_MOUSE, - STREQ(def->os.type, "hvm") ? - VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN, - { .alias = NULL }, + .type = VIR_DOMAIN_INPUT_TYPE_MOUSE, + .info = { .alias = NULL }, }; + if (STREQ(def->os.type, "hvm")) + autoInput.bus = VIR_DOMAIN_INPUT_BUS_PS2; + else if (STREQ(def->os.type, "xen")) + autoInput.bus = VIR_DOMAIN_INPUT_BUS_XEN; + else if (STREQ(def->os.type, "exe") && + def->virtType == VIR_DOMAIN_VIRT_PARALLELS) + autoInput.bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; + if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) goto error; -- 2.1.0

On 07.04.2015 22:35, Dmitry Guryanov wrote:
We should add input devices with proper bus, not VIR_DOMAIN_INPUT_BUS_XEN.
Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/conf/domain_conf.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8a6b84..da81e72 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21176,12 +21176,18 @@ virDomainDefFormatInternal(virDomainDefPtr def, /* If graphics is enabled, add the implicit mouse/keyboard */ if ((ARCH_IS_X86(def->os.arch)) || def->os.arch == VIR_ARCH_NONE) { virDomainInputDef autoInput = { - VIR_DOMAIN_INPUT_TYPE_MOUSE, - STREQ(def->os.type, "hvm") ? - VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN, - { .alias = NULL }, + .type = VIR_DOMAIN_INPUT_TYPE_MOUSE, + .info = { .alias = NULL }, };
+ if (STREQ(def->os.type, "hvm")) + autoInput.bus = VIR_DOMAIN_INPUT_BUS_PS2; + else if (STREQ(def->os.type, "xen")) + autoInput.bus = VIR_DOMAIN_INPUT_BUS_XEN; + else if (STREQ(def->os.type, "exe") && + def->virtType == VIR_DOMAIN_VIRT_PARALLELS) + autoInput.bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; + if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) goto error;
Unfortunately, other drivers rely on the XEN default. So I think this should look something like: if (hvm) bus = _PS2; else if (exe && PARALLELS) bus = PARALLELS; else bus = XEN; Michal

PCS doesn't store domain config in managed save state file. It's forbidden to change config for VMs in this state. It's possible to change config for containers, but after restoring domain will have that new config, not a config, which domain had at the moment of virDomainManagedSave. So we need to handle this case differently from other states. Let's forbid this operation, if config is changed and if it's not changed - just do nothing. Openstack/nova calls virDomainDefineXML on resume with current domain config, so we can't forbid this operation in managed save state. Signed-off-by: Dmitry Guryanov <dguryanov@parallels.com> --- src/parallels/parallels_driver.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index c3285db..1d852f5 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -728,11 +728,35 @@ parallelsDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int if (!olddom) goto cleanup; } else { - if (prlsdkApplyConfig(conn, olddom, def)) - goto cleanup; + int state, reason; + + state = virDomainObjGetState(olddom, &reason); + + if (state == VIR_DOMAIN_SHUTOFF && + reason == VIR_DOMAIN_SHUTOFF_SAVED) { + + /* PCS doesn't store domain config in managed save state file. + * It's forbidden to change config for VMs in this state. + * It's possible to change config for containers, but after + * restoring domain will have that new config, not a config, + * which domain had at the moment of virDomainManagedSave. + * + * So forbid this operation, if config is changed. If it's + * not changed - just do nothing. */ + + if (!virDomainDefCheckABIStability(olddom->def, def)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Can't change domain configuration " + "in managed save state")); + goto cleanup; + } + } else { + if (prlsdkApplyConfig(conn, olddom, def)) + goto cleanup; - if (prlsdkUpdateDomain(privconn, olddom)) - goto cleanup; + if (prlsdkUpdateDomain(privconn, olddom)) + goto cleanup; + } } retdom = virGetDomain(conn, def->name, def->uuid); -- 2.1.0

On 07.04.2015 22:34, Dmitry Guryanov wrote:
This patch series is intended to implement all needed code, so that suspend/resume in openstack nova will work.
It implements .domainHasManagedSaveImage, .domainManagedSave and .domainManagedSaveRemove functions.
It's not possible to change VM configuration, if it is in suspended state in PCS. It's possible for containers, but it has different meaning from the same action in libvirt: libvirt stores domain config in a save file and restores it if a user decided to start domain. But if the user discarded state file - domain will have new configuration.
PCS doesn't store domain config for containers, so it will have new configuration either if the user will start it from, save file or discard state file and start after that.
So the idea is to forbid changing domain configuration, if a domain is in managed saved state. We can't forbid calling this function in this state, because openstack/nova calls virDomainDefineXML in suspended state. So I compare current virDomainDef with the new one. And if there are no changes - function does nothing and returns success.
Dmitry Guryanov (14): parallels: fix headers in parallels_sdk.h parallels: split prlsdkDomainChangeState function parallels: implement virDomainManagedSave parallels: report, that cdroms are readonly parallels: add controllers in prlsdkLoadDomain parallels: fill adapter model in virDomainNetDef parallels: don't fill net adapter model for containers conf: add VIR_DOMAIN_VIDEO_TYPE_PARALLELS video type conf: return proper default video type for parallels conf: add input device type for parallels containers parallels: add implicit input devices conf: fix virDomainDefParseXML for parallels conf: fix virDomainDefFormatInternal for parallels parallels: fix virDomainDefineXML for domain in saved state
src/conf/domain_conf.c | 70 +++++++++++++++++++--- src/conf/domain_conf.h | 2 + src/parallels/parallels_driver.c | 104 ++++++++++++++++++++++++++++++-- src/parallels/parallels_sdk.c | 124 ++++++++++++++++++++++++++++++++++----- src/parallels/parallels_sdk.h | 17 ++++-- src/qemu/qemu_command.c | 6 +- 6 files changed, 285 insertions(+), 38 deletions(-)
I've updated the small nits I found, ACKed and pushed. Good job! Michal
participants (2)
-
Dmitry Guryanov
-
Michal Privoznik