Re: [libvirt] [PATCH 2/2] qemu: Set identity for the reconnect all thread
On Fri, Nov 09, 2018 at 19:39:37 -0500, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1631622
>
> If polkit authentication is enabled, an attempt to open
> the connection failed during virAccessDriverPolkitGetCaller
> when the call to virIdentityGetCurrent returned NULL resulting
> in the errors:
>
> virAccessDriverPolkitGetCaller:87 : access denied from:
> Policy kit denied action org.libvirt.api.connect.getattr from
>
> virAccessManagerSanitizeError:204 : access denied from: nwfilter
>
> Because qemuProcessReconnect runs in a thread during
> daemonRunStateInit processing it doesn't have the thread
> local identity. Thus when the virGetConnectNWFilter is
> called as part of the qemuProcessFiltersInstantiate when
> virDomainConfNWFilterInstantiate is run the attempt to get
> the idenity fails and results in the anonymous error above.
>
> To fix this, let's grab/use the virIdenityPtr of the process
> that will be creating the thread, e.g. what daemonRunStateInit
> has set and use that for our thread. That way any other similar
> processing that uses/requires an identity for any other call
> that would have previously been successfully run won't fail in
> a similar manner.
>
> Signed-off-by: John Ferlan
> ---
> src/qemu/qemu_process.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 06a65b44e4..93f6a2279a 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -81,6 +81,7 @@
> #include "netdev_bandwidth_conf.h"
> #include "virresctrl.h"
> #include "virvsock.h"
> +#include "viridentity.h"
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
> @@ -7716,6 +7717,7 @@ qemuProcessRefreshCPU(virQEMUDriverPtr driver,
> struct qemuProcessReconnectData {
> virQEMUDriverPtr driver;
> virDomainObjPtr obj;
> +virIdentityPtr identity;
> };
> /*
> * Open an existing VM's monitor, re-detect VCPU threads
> @@ -7753,6 +7755,7 @@ qemuProcessReconnect(void *opaque)
> bool retry = true;
> bool tryMonReconn = false;
>
> +virIdentitySetCurrent(data->identity);
This takes it's own reference to the identity. The reference in
data->identity is then leaked.
> VIR_FREE(data);
>
> qemuDomainObjRestoreJob(obj, );
> @@ -7979,6 +7982,7 @@ qemuProcessReconnect(void *opaque)
> virObjectUnref(cfg);
> virObjectUnref(caps);
> virNWFilterUnlockFilterUpdates();
> +virIdentitySetCurrent(NULL);
> return;
>
> error:
> @@ -8022,6 +8026,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
>
> memcpy(data, src, sizeof(*data));
> data->obj = obj;
> +data->identity = virIdentityGetCurrent();
In addition to the leak from the thread, the reference is also leaked if
the thread creation fails.
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: Fix VIR_ERR_ACCESS_DENIED formatting
On Fri, Nov 09, 2018 at 19:39:36 -0500, John Ferlan wrote:
> In commit ccc72d5cb I thought I had the brilliant idea that I
> could alter an error message in virErrorMsg to take two arguments
> "drivername" and NULL. Then passing the failed drivername for
> VIR_ERR_ACCESS_DENIED users would magically allow any "real"
> error message to be included. Of course it worked for the
> majority of errors, except for 1 teeny, tiny, problem - there
> was a caller using VIR_ERR_ACCESS_DENIED that actually passed
> "real" error message - virAccessDriverPolkitGetCaller, but
> didn't pass a driver name. So when this code went to call
> virErrorMsg it resulted in garbage being printed in that
> second argument because that's not how things are written.
>
> So in order to avoid that issue, reset the error back to the
> usual if info == NULL, the print just some text; otherwise,
> print a formatted output. For a majority of error messages
> this will print the driver name after "access deined from:".
> For the one case in question, the correct error message will
> be displayed and no garbage chars.
>
> Signed-off-by: John Ferlan
> ---
> src/util/virerror.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index 10f1b55c5f..df239bf371 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -1442,9 +1442,9 @@ virErrorMsg(virErrorNumber error, const char *info)
> break;
> case VIR_ERR_ACCESS_DENIED:
> if (info == NULL)
> -errmsg = _("access denied from '%s'");
> +errmsg = _("access denied");
> else
> -errmsg = _("access denied from '%s': %s");
> +errmsg = _("access denied from: %s");
> break;
The referenced patch modifies virAccessManagerSanitizeError which passes
a bogus NULL to virAccessError. Ideally the format string will be
changed to just "%s" so that 'driverName' can't be interpreted as a
format string.
Additionally the referenced patch also modifies src/rpc/gendispatch.pl
which also passes a bogus NULL to virReportError, which should be fixed
as well.
As a side-note, the original commit also attempted to print NULL
pointers with %s modifier with *printf which is undefined behavior.
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 10/10] news: Cold(un)plug and hot(un)plug support for usb hub
Signed-off-by: Han Han --- docs/news.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 88774c55ae..b677f52efc 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,16 @@ + + + Qemu: Add active and inactive device add or remove for QEMU + USB Hub devices + + + Add the ability to attach or detach a USB Hub device either + for active or inactive guests. + + -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 04/10] qemu: Make qemuBuildHubDevStr global
Make function qemuBuildHubDevStr global for reuse on hub hotplug. Signed-off-by: Han Han --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 4 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f59cbf559e..508f1b4477 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4675,7 +4675,7 @@ qemuBuildUSBHostdevDevStr(const virDomainDef *def, } -static char * +char * qemuBuildHubDevStr(const virDomainDef *def, virDomainHubDefPtr dev, virQEMUCapsPtr qemuCaps) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 98d4ac90b5..e1e4e56bc3 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -174,6 +174,10 @@ char *qemuBuildRedirdevDevStr(const virDomainDef *def, virDomainRedirdevDefPtr dev, virQEMUCapsPtr qemuCaps); +char *qemuBuildHubDevStr(const virDomainDef *def, + virDomainHubDefPtr dev, + virQEMUCapsPtr qemuCaps); + int qemuNetworkPrepareDevices(virDomainDefPtr def); int qemuGetDriveSourceString(virStorageSourcePtr src, -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 09/10] qemu: Implement usb hub device hotunplug
https://bugzilla.redhat.com/show_bug.cgi?id=1375423
Signed-off-by: Han Han
---
src/qemu/qemu_driver.c | 5 ++-
src/qemu/qemu_hotplug.c | 81 -
src/qemu/qemu_hotplug.h | 4 ++
3 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 774f6ac8b9..2813a00050 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7822,11 +7822,14 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
ret = qemuDomainDetachVsockDevice(vm, dev->data.vsock, async);
break;
+case VIR_DOMAIN_DEVICE_HUB:
+ret = qemuDomainDetachHubDevice(vm, dev->data.hub, async);
+break;
+
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_GRAPHICS:
-case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ca73456260..124703b7b2 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4965,6 +4965,32 @@ qemuDomainRemoveRedirdevDevice(virQEMUDriverPtr driver,
}
+static int
+qemuDomainRemoveHubDevice(virDomainObjPtr vm,
+ virDomainHubDefPtr dev)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverPtr driver = priv->driver;
+virObjectEventPtr event = NULL;
+size_t i;
+
+VIR_DEBUG("Removing hub device %s from domain %p %s",
+ dev->info.alias, vm, vm->def->name);
+
+event = virDomainEventDeviceRemovedNewFromObj(vm, dev->info.alias);
+virObjectEventStateQueue(driver->domainEventState, event);
+for (i = 0; i < vm->def->nhubs; i++) {
+if (vm->def->hubs[i] == dev)
+break;
+}
+qemuDomainReleaseDeviceAddress(vm, >info, NULL);
+
+virDomainHubDefFree(vm->def->hubs[i]);
+VIR_DELETE_ELEMENT(vm->def->hubs, i, vm->def->nhubs);
+return 0;
+}
+
+
int
qemuDomainRemoveDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -5016,13 +5042,16 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
ret = qemuDomainRemoveVsockDevice(vm, dev->data.vsock);
break;
+case VIR_DOMAIN_DEVICE_HUB:
+ret = qemuDomainRemoveHubDevice(vm, dev->data.hub);
+break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_GRAPHICS:
-case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
@@ -7019,3 +7048,53 @@ qemuDomainDetachVsockDevice(virDomainObjPtr vm,
qemuDomainResetDeviceRemoval(vm);
return ret;
}
+
+
+int
+qemuDomainDetachHubDevice(virDomainObjPtr vm,
+ virDomainHubDefPtr def,
+ bool async)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverPtr driver = priv->driver;
+virDomainHubDefPtr detach;
+int ret = -1;
+int idx;
+
+if ((idx = virDomainHubDefFind(vm->def, def)) < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("matching hub device not found"));
+return -1;
+}
+
+detach = vm->def->hubs[idx];
+if (qemuDomainHubIsBusy(vm, detach)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("device cannot be detached: device is busy"));
+goto cleanup;
+}
+
+if (!async)
+qemuDomainMarkDeviceForRemoval(vm, >info);
+
+qemuDomainObjEnterMonitor(driver, vm);
+if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) {
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+goto cleanup;
+}
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto cleanup;
+
+if (async) {
+ret = 0;
+} else {
+if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
+ret = qemuDomainRemoveHubDevice(vm, detach);
+}
+
+ cleanup:
+if (!async)
+qemuDomainResetDeviceRemoval(vm);
+return ret;
+}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 19b8950254..5c860c26ac 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -204,4 +204,8 @@ int qemuDomainDetachInputDevice(virDomainObjPtr vm,
int qemuDomainDetachVsockDevice(virDomainObjPtr vm,
virDomainVsockDefPtr dev,
bool async);
+
+int qemuDomainDetachHubDevice(virDomainObjPtr vm,
+ virDomainHubDefPtr def,
+ bool async);
#endif /* __QEMU_HOTPLUG_H__ */
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 00/10] Cold(un)plug and hot(un)plug support for hub
Changes from v2: - Add codes to check if hub is busy before hotunplug. (I am not sure if I covered all the usb devices in qemuDomainHubIsBusy. Please help to review it :) ) - Remove wrongly pasted codes - Update titles of commit msgs - Update news.xml v2: https://www.redhat.com/archives/libvir-list/2018-October/msg00719.html Han Han (10): qemu: Allow coldplugging of hub device qemu: Allow coldunplugging of hub device qemu: Refactor hub alias assignment for hotplug qemu: Make qemuBuildHubDevStr global conf: Add virDomainHubDefFree to libvirt_private.syms qemu: Implement usb hub device hotplug conf: Add function virDomainUSBAddressIsAttachedToHub qemu: Check whether hub device is busy qemu: Implement usb hub device hotunplug news: Cold(un)plug and hot(un)plug support for usb hub docs/news.xml| 10 ++ src/conf/domain_addr.c | 22 + src/conf/domain_addr.h | 5 + src/conf/domain_conf.c | 30 ++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 3 + src/qemu/qemu_alias.c| 22 - src/qemu/qemu_alias.h| 4 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 4 + src/qemu/qemu_driver.c | 30 +- src/qemu/qemu_hotplug.c | 203 ++- src/qemu/qemu_hotplug.h | 8 ++ 13 files changed, 336 insertions(+), 10 deletions(-) -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 05/10] conf: Add virDomainHubDefFree to libvirt_private.syms
Add virDomainHubDefFree to for the preparation of usb hub hotplug. Signed-off-by: Han Han --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6245927673..b29c2bf62b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -400,6 +400,7 @@ virDomainHostdevSubsysPCIBackendTypeToString; virDomainHostdevSubsysTypeToString; virDomainHPTResizingTypeToString; virDomainHubDefFind; +virDomainHubDefFree; virDomainHubTypeFromString; virDomainHubTypeToString; virDomainHypervTypeFromString; -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 08/10] qemu: Check whether hub device is busy
qemuDomainHubIsBusy is to check whether a usb device are attached to the hub
device. It will be used for hotunplugging and live device update of hub
device.
Signed-off-by: Han Han
---
src/qemu/qemu_hotplug.c | 64 +
1 file changed, 64 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1b6cc36bc8..ca73456260 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5332,6 +5332,70 @@ static bool qemuDomainControllerIsBusy(virDomainObjPtr
vm,
}
}
+static bool qemuDomainHubIsBusy(virDomainObjPtr vm,
+virDomainHubDefPtr detach)
+{
+size_t i;
+virDomainDiskDefPtr disk;
+virDomainHubDefPtr hub;
+virDomainInputDefPtr input;
+virDomainSoundDefPtr sound;
+virDomainHostdevDefPtr hostdev;
+virDomainRedirdevDefPtr redirdev;
+virDomainControllerDefPtr controller;
+
+for (i = 0; i < vm->def->ndisks; i++) {
+disk = vm->def->disks[i];
+if (disk->bus == VIR_DOMAIN_DISK_BUS_USB &&
+virDomainUSBAddressIsAttachedToHub(&(disk->info), detach))
+return true;
+}
+
+for (i = 0; i < vm->def->nhubs; i++) {
+hub = vm->def->hubs[i];
+if (virDomainUSBAddressIsAttachedToHub(&(hub->info), detach))
+return true;
+}
+
+for (i = 0; i < vm->def->ninputs; i++) {
+input = vm->def->inputs[i];
+if (input->bus == VIR_DOMAIN_INPUT_BUS_USB &&
+virDomainUSBAddressIsAttachedToHub(&(input->info), detach))
+return true;
+}
+
+for (i = 0; i < vm->def->nsounds; i++) {
+sound = vm->def->sounds[i];
+if (sound->model == VIR_DOMAIN_SOUND_MODEL_USB &&
+virDomainUSBAddressIsAttachedToHub(&(sound->info), detach))
+return true;
+}
+
+for (i = 0; i < vm->def->nhostdevs; i++) {
+hostdev = vm->def->hostdevs[i];
+if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB
&&
+virDomainUSBAddressIsAttachedToHub(hostdev->info, detach))
+return true;
+}
+
+for (i = 0; i < vm->def->nredirdevs; i++) {
+redirdev = vm->def->redirdevs[i];
+if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB &&
+virDomainUSBAddressIsAttachedToHub(&(redirdev->info), detach))
+return true;
+}
+
+for (i = 0; i < vm->def->ncontrollers; i++) {
+controller = vm->def->controllers[i];
+if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_CCID &&
+virDomainUSBAddressIsAttachedToHub(&(controller->info), detach))
+return false;
+}
+
+return false;
+}
+
int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev,
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 03/10] qemu: Refactor hub alias assignment for hotplug
Make qemuAssignDeviceHubAlias global and allow alias
generating for reuse on hotplug.
Signed-off-by: Han Han
---
src/qemu/qemu_alias.c | 22 ++
src/qemu/qemu_alias.h | 4
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 815caec465..116480eaee 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -364,14 +364,28 @@ qemuAssignDeviceVideoAlias(virDomainVideoDefPtr video,
}
-static int
-qemuAssignDeviceHubAlias(virDomainHubDefPtr hub,
+int
+qemuAssignDeviceHubAlias(virDomainDefPtr def,
+ virDomainHubDefPtr hub,
int idx)
{
if (hub->info.alias)
return 0;
-return virAsprintf(>info.alias, "hub%d", idx);
+if (idx == -1) {
+int thisidx;
+size_t i;
+
+for (i = 0; i < def->nhubs; i++) {
+if ((thisidx = qemuDomainDeviceAliasIndex(>hubs[i]->info,
"hub")) >= idx)
+idx = thisidx + 1;
+}
+}
+
+if (virAsprintf(>info.alias, "hub%d", idx) < 0)
+return -1;
+
+return 0;
}
@@ -647,7 +661,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr
qemuCaps)
return -1;
}
for (i = 0; i < def->nhubs; i++) {
-if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0)
+if (qemuAssignDeviceHubAlias(def, def->hubs[i], i) < 0)
return -1;
}
for (i = 0; i < def->nshmems; i++) {
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 33b9937ea4..ea30c1c8a3 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -71,6 +71,10 @@ int qemuAssignDeviceInputAlias(virDomainDefPtr def,
virDomainInputDefPtr input,
int idx);
+int qemuAssignDeviceHubAlias(virDomainDefPtr def,
+ virDomainHubDefPtr hub,
+ int idx);
+
int qemuAssignDeviceVsockAlias(virDomainVsockDefPtr vsock);
int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps);
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 07/10] conf: Add function virDomainUSBAddressIsAttachedToHub
Add this function to check if the a usb address is attached to a hub device.
Signed-off-by: Han Han
---
src/conf/domain_addr.c | 22 ++
src/conf/domain_addr.h | 5 +
src/libvirt_private.syms | 1 +
3 files changed, 28 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index e4ed143b76..722bd2c9fe 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -2155,6 +2155,28 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr
addrs,
}
+bool
+virDomainUSBAddressIsAttachedToHub(virDomainDeviceInfoPtr info,
+ virDomainHubDefPtr hub)
+{
+unsigned int *hub_port = hub->info.addr.usb.port;
+unsigned int *device_port = info->addr.usb.port;
+size_t i;
+if (hub->info.addr.usb.bus == info->addr.usb.bus) {
+for (i = 0; i < VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH; ++i) {
+if (hub_port[i] == device_port[i])
+continue;
+else if (hub_port[i] == 0 && device_port[i] != 0)
+return true;
+else
+return false;
+}
+}
+
+return false;
+}
+
+
int
virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
virDomainDeviceInfoPtr info)
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 2a9af9c00b..b1e0714923 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -285,6 +285,11 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
virDomainDeviceInfoPtr info)
ATTRIBUTE_NONNULL(2);
+bool
+virDomainUSBAddressIsAttachedToHub(virDomainDeviceInfoPtr info,
+ virDomainHubDefPtr hub)
+ATTRIBUTE_NONNULL(2);
+
int
virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
virDomainDeviceInfoPtr info)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b29c2bf62b..b45a7b92b4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -131,6 +131,7 @@ virDomainPCIControllerModelToConnectType;
virDomainUSBAddressAssign;
virDomainUSBAddressCountAllPorts;
virDomainUSBAddressEnsure;
+virDomainUSBAddressIsAttachedToHub;
virDomainUSBAddressPortFormatBuf;
virDomainUSBAddressPortIsValid;
virDomainUSBAddressPresent;
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 01/10] qemu: Allow coldplugging of hub device
https://bugzilla.redhat.com/show_bug.cgi?id=1375423
Signed-off-by: Han Han
---
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9f71641dfa..1e5a69358b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8133,6 +8133,11 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
return -1;
break;
+case VIR_DOMAIN_DEVICE_HUB:
+if (VIR_APPEND_ELEMENT(vmdef->hubs, vmdef->nhubs, dev->data.hub) < 0)
+return -1;
+break;
+
case VIR_DOMAIN_DEVICE_VSOCK:
if (vmdef->vsock) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -8145,7 +8150,6 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_GRAPHICS:
-case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 02/10] qemu: Allow coldunplugging of hub device
https://bugzilla.redhat.com/show_bug.cgi?id=1375423
Signed-off-by: Han Han
---
src/conf/domain_conf.c | 30 ++
src/conf/domain_conf.h | 3 +++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 10 +-
4 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 237540bccc..a2655bc29b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17652,6 +17652,36 @@ virDomainVsockDefEquals(const virDomainVsockDef *a,
}
+static bool
+virDomainHubDefEquals(const virDomainHubDef *a,
+ const virDomainHubDef *b)
+{
+if (a->type != b->type)
+return false;
+
+if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+!virDomainDeviceInfoAddressIsEqual(>info, >info))
+return false;
+
+return true;
+}
+
+
+ssize_t
+virDomainHubDefFind(const virDomainDef *def,
+const virDomainHubDef *hub)
+{
+size_t i;
+
+for (i = 0; i < def->nhubs; i++) {
+if (virDomainHubDefEquals(hub, def->hubs[i]))
+return i;
+}
+
+return -1;
+}
+
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e30a4b2fe7..c2d0877170 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3365,6 +3365,9 @@ virDomainShmemDefPtr
virDomainShmemDefRemove(virDomainDefPtr def, size_t idx)
ssize_t virDomainInputDefFind(const virDomainDef *def,
const virDomainInputDef *input)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ssize_t virDomainHubDefFind(const virDomainDef *def,
+ const virDomainHubDef *hub)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
bool virDomainVsockDefEquals(const virDomainVsockDef *a,
const virDomainVsockDef *b)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 335210c31d..6245927673 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -399,6 +399,7 @@ virDomainHostdevRemove;
virDomainHostdevSubsysPCIBackendTypeToString;
virDomainHostdevSubsysTypeToString;
virDomainHPTResizingTypeToString;
+virDomainHubDefFind;
virDomainHubTypeFromString;
virDomainHubTypeToString;
virDomainHypervTypeFromString;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e5a69358b..4209f017c7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8338,10 +8338,18 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
vmdef->vsock = NULL;
break;
+case VIR_DOMAIN_DEVICE_HUB:
+if ((idx = virDomainHubDefFind(vmdef, dev->data.hub)) < 0) {
+virReportError(VIR_ERR_DEVICE_MISSING, "%s",
+ _("matching hub device not found"));
+return -1;
+}
+VIR_DELETE_ELEMENT(vmdef->hubs, idx, vmdef->nhubs);
+break;
+
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_GRAPHICS:
-case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 06/10] qemu: Implement usb hub device hotplug
https://bugzilla.redhat.com/show_bug.cgi?id=1375423
Signed-off-by: Han Han
---
src/qemu/qemu_driver.c | 9 ++-
src/qemu/qemu_hotplug.c | 58 +
src/qemu/qemu_hotplug.h | 4 +++
3 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4209f017c7..774f6ac8b9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7710,12 +7710,19 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
}
break;
+case VIR_DOMAIN_DEVICE_HUB:
+ret = qemuDomainAttachHubDevice(driver, vm, dev->data.hub);
+if (ret == 0) {
+alias = dev->data.hub->info.alias;
+dev->data.hub = NULL;
+}
+break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_GRAPHICS:
-case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0a63741b9e..1b6cc36bc8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3303,6 +3303,64 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver,
}
+int
+qemuDomainAttachHubDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainHubDefPtr hub)
+{
+int ret = -1;
+char *devstr = NULL;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virErrorPtr originalError = NULL;
+bool releaseaddr = false;
+
+if (priv->usbaddrs) {
+if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
+goto cleanup;
+releaseaddr = true;
+}
+
+if (qemuAssignDeviceHubAlias(vm->def, hub, -1) < 0)
+goto cleanup;
+
+if (!(devstr = qemuBuildHubDevStr(vm->def, hub, priv->qemuCaps)))
+goto cleanup;
+
+if (VIR_REALLOC_N(vm->def->hubs, vm->def->nhubs + 1) < 0)
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+goto exit_monitor;
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+releaseaddr = false;
+goto cleanup;
+}
+
+VIR_APPEND_ELEMENT_COPY_INPLACE(vm->def->hubs, vm->def->nhubs, hub);
+
+ret = 0;
+releaseaddr = false;
+
+ cleanup:
+if (ret < 0) {
+virErrorPreserveLast();
+if (releaseaddr)
+qemuDomainReleaseDeviceAddress(vm, >info, NULL);
+virErrorRestore();
+}
+
+VIR_FREE(devstr);
+return ret;
+
+ exit_monitor:
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+releaseaddr = false;
+goto cleanup;
+}
+
+
int
qemuDomainAttachVsockDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 0297e42a98..19b8950254 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -135,6 +135,10 @@ int qemuDomainAttachInputDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainInputDefPtr input);
+int qemuDomainAttachHubDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainHubDefPtr hub);
+
int qemuDomainAttachVsockDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainVsockDefPtr vsock);
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 00/37] BaselineHypervisorCPU using QEMU QMP exchanges
Quoting Collin Walling (2018-11-09 10:44:41) > Hi Chris, > > On 11/9/18 11:27 AM, Chris Venteicher wrote: > > Quoting Chris Venteicher (2018-11-02 22:13:01) > >> Some architectures (S390) depend on QEMU to compute baseline CPU model and > >> expand a models feature set. > >> > >> Interacting with QEMU requires starting the QEMU process and completing > >> one or > >> more query-cpu-model-baseline QMP exchanges with QEMU in addition to a > >> query-cpu-model-expansion QMP exchange to expand all features in the model. > >> > >> See "s390x CPU models: exposing features" patch set on Qemu-devel for > >> discussion > >> of QEMU aspects. > >> > >> This is part of resolution of: > >> https://bugzilla.redhat.com/show_bug.cgi?id=1511999 > >> > >> Version 4 attempts to address all issues from V1,2,3 including making the > >> QEMU process activation for QMP Queries generic (not specific to > >> capabilities) > >> and moving that code from qemu_capabilities to qemu_process. > >> > >> Version 4 greatly expands the number of patches to make the set easier > >> to review. > >> > >> The patches to do hypervisor baseline using QEMU are primarily in > >> qemu_driver.c > >> > > Patches 1-2 create the cpumodel to/from Json conversion code. > > Patches 3-4 modify the cpu expansion interface. > > Patches 28-36 are the actual baseline changes. > > > >> The patches to make the process code generic (not capabilities specific) > >> are mostly in qemu_process.c > > > > Patches 5 -> 27 move the process code from qemu_capabilities to > > qemu_process. > > > > A lot of these are code move or rename patches so the patches with real > > implementation changes are easy to identify. > > > > I might have ended up with too many patches though. > > Want to mention... the whole "process" block could be moved to it's own > > series if that would be easier to review. > > I've been meaning to provide an in-depth review of these patches, but other > things > have been holding me up -- my apologies. > > At a quick glance, a lot of your patches involve a few short patches that > don't > necessarily provide much context on their own. A lot of patches in this > series can > definitely be merged. > > I think moving the "qemu process" code into its own series would be helpful. > I would > include something in the cover that they will benefit the hypervisor CPU > baseline and > comparison patches. If you feel confident with your qemu process patches, > then you > could also send your baseline patch series at the same time, and state that > your > baseline patches rely on your QEMU process patches (make sure to include a > mailing > list archive link in that header so reviewers can easily reference the other > patch > series). > > Also, make sure you CC the authors and contributors of the respective files > that you > touch. Most of them are listed at the top of the file. (I think Jiri might be > interested > in this series as well.) > > So let's start there. Repost your qemu_process code as it's own series, and > I'd recommend > tagging it with "RFC" (Request For Comments) instead of giving it a version > number. This > will prompt reviewers that you're mainly looking for areas of improvement and > some guidance. > Thanks Collin, The process stuff has been resubmitted in this patch series: [PATCH RFC 00/22] Move process code to qemu_process The rest of this series can be resubmitted when the process changes are solid. > > > >> Many of the patches are cut / paste of existing code. The patches that > >> change functionality are as modular and minimal as possible to make > >> reviewing easier. > >> > >> I attempted to make the new qemu_process functions > >> consistent with the existing domain activation qemu_process functions. > >> > >> A thread safe library function creates a unique directory under libDir for > >> each QEMU > >> process (for QMP messaging) to decouple processes in terms of sockets and > >> file system footprint. > >> > >> The last patch is based on past discussion of QEMU cpu > >> expansion only returning migratable features except for one x86 case where > >> non-migratable features are explicitly requested. The patch records that > >> features > >> are migratable based on QEMU only returning migratable features. The main > >> result > >> is the CPU xml for S390 CPU's contains the same migratability info the X86 > >> currently > >> does. The testcases were updated to reflect the change. > >> > >> Every patch should compile independently if applied in sequence. > >> > >> Thanks, > >> Chris > >> > >> > >> Chris Venteicher (37): > >> qemu_monitor: Introduce qemuMonitorCPUModelInfoNew > >> qemu_monitor: Introduce qemuMonitorCPUModelInfo / JSON conversion > >> qemu_capabilities: Introduce virQEMuCapsMigratablePropsDiff > >> qemu_monitor: qemuMonitorGetCPUModelExpansion inputs and outputs > >> CPUModelInfo > >> qemu_process: Move process code from qemu_capabilities to qemu_process >
[libvirt] [PATCH RFC 20/22] qemu_process: Enter QMP command mode when starting QEMU Process
qemuProcessStartQmp starts a QEMU process and monitor connection that
can be used by multiple functions possibly for multiple QMP commands.
The QMP exchange to exit capabilities negotiation mode and enter command mode
can only be performed once after the monitor connection is established.
Move responsibility for entering QMP command mode into the qemuProcess
code so multiple functions can issue QMP commands in arbitrary orders.
This also simplifies the functions using the connection provided by
qemuProcessStartQmp to issue QMP commands.
Test code now needs to call qemuMonitorSetCapabilities to send the
message to switch to command mode because the test code does not use the
qemuProcess command that internally calls qemuMonitorSetCapabilities.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 14 --
src/qemu/qemu_process.c | 8
tests/qemucapabilitiestest.c | 7 +++
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7168c470f6..f06d0a4428 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4013,13 +4013,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
/* @mon is supposed to be locked by callee */
-if (qemuMonitorSetCapabilities(mon) < 0) {
-VIR_DEBUG("Failed to set monitor capabilities %s",
- virGetLastErrorMessage());
-ret = 0;
-goto cleanup;
-}
-
if (qemuMonitorGetVersion(mon,
, , ,
) < 0) {
@@ -4193,13 +4186,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
{
int ret = -1;
-if (qemuMonitorSetCapabilities(mon) < 0) {
-VIR_DEBUG("Failed to set monitor capabilities %s",
- virGetLastErrorMessage());
-ret = 0;
-goto cleanup;
-}
-
if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, true) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d20f832053..eba2cd6765 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8298,6 +8298,14 @@ qemuConnectMonitorQmp(qemuProcessPtr proc)
virObjectLock(proc->mon);
+/* Exit capabilities negotiation mode and enter QEMU command mode
+ * by issuing qmp_capabilities command to QEMU */
+if (qemuMonitorSetCapabilities(proc->mon) < 0) {
+VIR_DEBUG("Failed to set monitor capabilities %s",
+ virGetLastErrorMessage());
+goto ignore;
+}
+
ignore:
ret = 0;
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index 8fe5a55e1d..ea4671739b 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -58,6 +58,9 @@ testQemuCaps(const void *opaque)
if (!(mon = qemuMonitorTestNewFromFileFull(repliesFile, >driver,
NULL)))
goto cleanup;
+if (qemuMonitorSetCapabilities(qemuMonitorTestGetMonitor(mon)) < 0)
+goto cleanup;
+
if (!(capsActual = virQEMUCapsNew()) ||
virQEMUCapsInitQMPMonitor(capsActual,
qemuMonitorTestGetMonitor(mon)) < 0)
@@ -65,6 +68,10 @@ testQemuCaps(const void *opaque)
if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) {
qemuMonitorResetCommandID(qemuMonitorTestGetMonitor(mon));
+
+if (qemuMonitorSetCapabilities(qemuMonitorTestGetMonitor(mon)) < 0)
+goto cleanup;
+
if (virQEMUCapsInitQMPMonitorTCG(capsActual,
qemuMonitorTestGetMonitor(mon)) < 0)
goto cleanup;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 16/22] qemu_process: Cleanup qemuProcess alloc function
qemuProcessNew is one of the 4 public functions used to create and
manage a qemu process for QMP command exchanges outside of domain
operations.
Add descriptive comment block, Debug statement and make source
consistent with the cleanup / VIR_STEAL_PTR format used elsewhere.
No functional changes are made.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 55a092ecbb..5ff7d6878c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8097,6 +8097,18 @@ qemuProcessFree(qemuProcessPtr proc)
}
+/**
+ * qemuProcessNew:
+ * @binary: Qemu binary
+ * @libDir: Directory for process and connection artifacts
+ * @runUid: UserId for Qemu Process
+ * @runGid: GroupId for Qemu Process
+ * @forceTCG: Force TCG mode if true
+ *
+ * Allocate and initialize domain structure encapsulating
+ * QEMU Process state and monitor connection to QEMU
+ * for completing QMP Queries.
+ */
qemuProcessPtr
qemuProcessNew(const char *binary,
const char *libDir,
@@ -8104,25 +8116,29 @@ qemuProcessNew(const char *binary,
gid_t runGid,
bool forceTCG)
{
+qemuProcessPtr ret = NULL;
qemuProcessPtr proc = NULL;
+VIR_DEBUG("exec=%s, libDir=%s, runUid=%u, runGid=%u, forceTCG=%d",
+ NULLSTR(binary), NULLSTR(libDir), runUid, runGid, forceTCG);
+
if (VIR_ALLOC(proc) < 0)
-goto error;
+goto cleanup;
if (VIR_STRDUP(proc->binary, binary) < 0 ||
VIR_STRDUP(proc->libDir, libDir) < 0)
-goto error;
+goto cleanup;
proc->forceTCG = forceTCG;
proc->runUid = runUid;
proc->runGid = runGid;
-return proc;
+VIR_STEAL_PTR(ret, proc);
- error:
+ cleanup:
qemuProcessFree(proc);
-return NULL;
+return ret;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 14/22] qemu_process: Stop retaining Qemu Monitor config in qemuProcess
The monitor config data is removed from the qemuProcess struct.
The monitor config data can be initialized immediately before call to
qemuMonitorOpen and does not need to be maintained after the call
because qemuMonitorOpen copies any strings it needs.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 9 +
src/qemu/qemu_process.h | 1 -
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8e98b97443..d4ed2d6353 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8237,13 +8237,14 @@ qemuConnectMonitorQmp(qemuProcessPtr proc)
{
int ret = -1;
virDomainXMLOptionPtr xmlopt = NULL;
+virDomainChrSourceDef monConfig;
VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
proc, NULLSTR(proc->binary), (long long) proc->pid);
-proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-proc->config.data.nix.path = proc->monpath;
-proc->config.data.nix.listen = false;
+monConfig.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+monConfig.data.nix.path = proc->monpath;
+monConfig.data.nix.listen = false;
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
!(proc->vm = virDomainObjNew(xmlopt)))
@@ -8251,7 +8252,7 @@ qemuConnectMonitorQmp(qemuProcessPtr proc)
proc->vm->pid = proc->pid;
-if (!(proc->mon = qemuMonitorOpen(proc->vm, >config, true, true,
+if (!(proc->mon = qemuMonitorOpen(proc->vm, , true, true,
0, , NULL)))
goto ignore;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 1b8f743861..d1541d5407 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -227,7 +227,6 @@ struct _qemuProcess {
char *pidfile;
virCommandPtr cmd;
qemuMonitorPtr mon;
-virDomainChrSourceDef config;
pid_t pid;
virDomainObjPtr vm;
bool forceTCG;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 22/22] qemu_process: Stop locking QMP process monitor immediately
Locking the monitor object immediately after call to qemuMonitorOpen
doesn't make sense now that we have expanded the QEMU process code to
cover more than the original capabilities usecase.
Removing the monitor lock makes the qemuConnectMonitorQmp code
consistent with the qemuConnectMonitor code used to establish the
monitor when QEMU process is started for domains.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4dbc7038fd..2f9c1701a3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8297,8 +8297,6 @@ qemuConnectMonitorQmp(qemuProcessPtr proc)
VIR_STEAL_PTR(proc->mon, mon);
-virObjectLock(proc->mon);
-
/* Exit capabilities negotiation mode and enter QEMU command mode
* by issuing qmp_capabilities command to QEMU */
if (qemuMonitorSetCapabilities(proc->mon) < 0) {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 19/22] qemu_monitor: Make monitor callbacks optional
Qemu process code for capababilities doesn't use monitor callbacks and
defines empty callback functions.
Allow NULL to be passed to qemuMonitorOpen for callbacks and remove the
empty functions from the QMP process code.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_process.c | 14 +-
2 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7f7013e115..77f4fe7cf7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -813,12 +813,12 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
{
qemuMonitorPtr mon;
-if (!cb->eofNotify) {
+if (cb && !cb->eofNotify) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("EOF notify callback must be supplied"));
return NULL;
}
-if (!cb->errorNotify) {
+if (cb && !cb->errorNotify) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Error notify callback must be supplied"));
return NULL;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c44e46fc88..d20f832053 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8066,18 +8066,6 @@ qemuProcessReconnectAll(virQEMUDriverPtr driver)
}
-static void virQEMUCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
- virDomainObjPtr vm ATTRIBUTE_UNUSED,
- void *opaque ATTRIBUTE_UNUSED)
-{
-}
-
-static qemuMonitorCallbacks callbacks = {
-.eofNotify = virQEMUCapsMonitorNotify,
-.errorNotify = virQEMUCapsMonitorNotify,
-};
-
-
/**
* qemuProcessFree:
* @proc: Stores Process and Connection State
@@ -8270,7 +8258,7 @@ qemuConnectMonitorQmp(qemuProcessPtr proc)
bool retry = true;
bool enableJson = true;
virQEMUDriverPtr driver = NULL;
-qemuMonitorCallbacksPtr monCallbacks =
+qemuMonitorCallbacksPtr monCallbacks = NULL;
virDomainXMLOptionPtr xmlopt = NULL;
virDomainChrSourceDef monConfig;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 21/22] qemu_process: Use unique directories for QMP processes
Multiple QEMU processes for QMP commands can operate concurrently.
Use a unique directory under libDir for each QEMU processes
to avoid pidfile and unix socket collision between processes.
The pid file name is changed from "capabilities.pidfile" to "qmp.pid"
because we no longer need to avoid a possible clash with a qemu domain
called "capabilities" now that the processes artifacts are stored in
their own unique temporary directories.
"Capabilities" was changed to "qmp" in the pid file name because these
processes are no longer specific to the capabilities usecase and are
more generic in terms of being used for any general purpose QMP message
exchanges with a QEMU process that is not associated with a domain.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 26 --
src/qemu/qemu_process.h | 1 +
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index eba2cd6765..4dbc7038fd 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8090,6 +8090,7 @@ qemuProcessFree(qemuProcessPtr proc)
VIR_FREE(proc->binary);
VIR_FREE(proc->libDir);
+VIR_FREE(proc->uniqDir);
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
VIR_FREE(proc->pidfile);
@@ -8148,33 +8149,33 @@ qemuProcessNew(const char *binary,
static int
qemuProcessInitQmp(qemuProcessPtr proc)
{
+char *template = NULL;
int ret = -1;
VIR_DEBUG("Beginning VM startup process"
"emulator=%s",
proc->binary);
-/* the ".sock" sufix is important to avoid a possible clash with a qemu
- * domain called "capabilities"
- */
-if (virAsprintf(>monpath, "%s/%s", proc->libDir,
-"capabilities.monitor.sock") < 0)
+if (virAsprintf(, "%s/qemu.XX", proc->libDir) < 0)
+goto cleanup;
+
+proc->uniqDir = mkdtemp(template);
+
+if (virAsprintf(>monpath, "%s/%s", proc->uniqDir,
+"qmp.monitor") < 0)
goto cleanup;
if (virAsprintf(>monarg, "unix:%s,server,nowait", proc->monpath) < 0)
goto cleanup;
-/* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
- * with a qemu domain called "capabilities"
+/*
* Normally we'd use runDir for pid files, but because we're using
* -daemonize we need QEMU to be allowed to create them, rather
* than libvirtd. So we're using libDir which QEMU can write to
*/
-if (virAsprintf(>pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
+if (virAsprintf(>pidfile, "%s/%s", proc->uniqDir, "qmp.pid") < 0)
goto cleanup;
-virPidFileForceCleanupPath(proc->pidfile);
-
ret = 0;
cleanup:
@@ -8415,4 +8416,9 @@ void qemuProcessStopQmp(qemuProcessPtr proc)
unlink(proc->pidfile);
proc->pid = 0;
+
+
+if (proc->uniqDir)
+rmdir(proc->uniqDir);
+
}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index d1541d5407..f66fc0c82c 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -225,6 +225,7 @@ struct _qemuProcess {
char *monarg;
char *monpath;
char *pidfile;
+char *uniqDir;
virCommandPtr cmd;
qemuMonitorPtr mon;
pid_t pid;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 07/22] qemu_process: Use qemuProcess struct for a single process
In new process code, move from model where qemuProcess struct can be
used to activate a series of Qemu processes to model where one
qemuProcess struct is used for one and only one Qemu process.
The forceTCG parameter (use / don't use KVM) will be passed when the
qemuProcess struct is initialized since the qemuProcess struct won't be
reused.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 16
src/qemu/qemu_process.c | 11 +++
src/qemu/qemu_process.h | 6 --
3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 082874082b..a957c3bdbd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4220,14 +4220,16 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
char **qmperr)
{
qemuProcessPtr proc = NULL;
+qemuProcessPtr proc_kvm = NULL;
int ret = -1;
int rc;
+bool forceTCG = false;
if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
-runUid, runGid, qmperr)))
+runUid, runGid, qmperr, forceTCG)))
goto cleanup;
-if ((rc = qemuProcessRun(proc, false)) != 0) {
+if ((rc = qemuProcessRun(proc)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
@@ -4238,13 +4240,17 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
qemuProcessStopQmp(proc);
-if ((rc = qemuProcessRun(proc, true)) != 0) {
+
+forceTCG = true;
+proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid,
NULL, forceTCG);
+
+if ((rc = qemuProcessRun(proc_kvm)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
}
-if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon) < 0)
+if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc_kvm->mon) < 0)
goto cleanup;
}
@@ -4252,7 +4258,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
cleanup:
qemuProcessStopQmp(proc);
+qemuProcessStopQmp(proc_kvm);
qemuProcessFree(proc);
+qemuProcessFree(proc_kvm);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2571024e8e..dda74d5b7a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8100,7 +8100,8 @@ qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr)
+ char **qmperr,
+ bool forceTCG)
{
qemuProcessPtr proc = NULL;
@@ -8110,10 +8111,13 @@ qemuProcessNew(const char *binary,
if (VIR_STRDUP(proc->binary, binary) < 0)
goto error;
+proc->forceTCG = forceTCG;
+
proc->runUid = runUid;
proc->runGid = runGid;
proc->qmperr = qmperr;
+
/* the ".sock" sufix is important to avoid a possible clash with a qemu
* domain called "capabilities"
*/
@@ -8151,15 +8155,14 @@ qemuProcessNew(const char *binary,
* 1 when probing QEMU failed
*/
int
-qemuProcessRun(qemuProcessPtr proc,
- bool forceTCG)
+qemuProcessRun(qemuProcessPtr proc)
{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
int status = 0;
int ret = -1;
-if (forceTCG)
+if (proc->forceTCG)
machine = "none,accel=tcg";
else
machine = "none,accel=kvm:tcg";
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 25343c4592..ab2640ce7c 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -229,17 +229,19 @@ struct _qemuProcess {
virDomainChrSourceDef config;
pid_t pid;
virDomainObjPtr vm;
+bool forceTCG;
};
qemuProcessPtr qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr);
+ char **qmperr,
+ bool forceTCG);
void qemuProcessFree(qemuProcessPtr proc);
-int qemuProcessRun(qemuProcessPtr proc, bool forceTCG);
+int qemuProcessRun(qemuProcessPtr proc);
void qemuProcessStopQmp(qemuProcessPtr proc);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 13/22] qemu_process: Setup paths within qemuProcessInitQmp
Move code for setting paths and prepping file system from qemuProcessNew
to qemuProcessInitQmp.
This keeps qemuProcessNew limited to structure initialization and most
closely mirrors pattern established in qemuProcessInit within
qemuProcessInitQmp.
The patch is a non-functional, cut / paste change,
however goto is now "cleanup" rather than "error".
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 42 +
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0b96debf25..8e98b97443 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8118,26 +8118,6 @@ qemuProcessNew(const char *binary,
proc->runUid = runUid;
proc->runGid = runGid;
-/* the ".sock" sufix is important to avoid a possible clash with a qemu
- * domain called "capabilities"
- */
-if (virAsprintf(>monpath, "%s/%s", proc->libDir,
-"capabilities.monitor.sock") < 0)
-goto error;
-if (virAsprintf(>monarg, "unix:%s,server,nowait", proc->monpath) < 0)
-goto error;
-
-/* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
- * with a qemu domain called "capabilities"
- * Normally we'd use runDir for pid files, but because we're using
- * -daemonize we need QEMU to be allowed to create them, rather
- * than libvirtd. So we're using libDir which QEMU can write to
- */
-if (virAsprintf(>pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
-goto error;
-
-virPidFileForceCleanupPath(proc->pidfile);
-
return proc;
error:
@@ -8157,8 +8137,30 @@ qemuProcessInitQmp(qemuProcessPtr proc)
"emulator=%s",
proc->binary);
+/* the ".sock" sufix is important to avoid a possible clash with a qemu
+ * domain called "capabilities"
+ */
+if (virAsprintf(>monpath, "%s/%s", proc->libDir,
+"capabilities.monitor.sock") < 0)
+goto cleanup;
+
+if (virAsprintf(>monarg, "unix:%s,server,nowait", proc->monpath) < 0)
+goto cleanup;
+
+/* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
+ * with a qemu domain called "capabilities"
+ * Normally we'd use runDir for pid files, but because we're using
+ * -daemonize we need QEMU to be allowed to create them, rather
+ * than libvirtd. So we're using libDir which QEMU can write to
+ */
+if (virAsprintf(>pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
+goto cleanup;
+
+virPidFileForceCleanupPath(proc->pidfile);
+
ret = 0;
+ cleanup:
VIR_DEBUG("ret=%i", ret);
return ret;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 12/22] qemu_process: Store libDir in qemuProcess struct
Store libDir path in the qemuProcess struct in anticipation of moving
path construction code into qemuProcessInitQmp function.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 8 +---
src/qemu/qemu_process.h | 1 +
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fe4361ed5d..0b96debf25 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8088,6 +8088,7 @@ qemuProcessFree(qemuProcessPtr proc)
qemuProcessStopQmp(proc);
VIR_FREE(proc->binary);
+VIR_FREE(proc->libDir);
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
VIR_FREE(proc->pidfile);
@@ -8108,7 +8109,8 @@ qemuProcessNew(const char *binary,
if (VIR_ALLOC(proc) < 0)
goto error;
-if (VIR_STRDUP(proc->binary, binary) < 0)
+if (VIR_STRDUP(proc->binary, binary) < 0 ||
+VIR_STRDUP(proc->libDir, libDir) < 0)
goto error;
proc->forceTCG = forceTCG;
@@ -8119,7 +8121,7 @@ qemuProcessNew(const char *binary,
/* the ".sock" sufix is important to avoid a possible clash with a qemu
* domain called "capabilities"
*/
-if (virAsprintf(>monpath, "%s/%s", libDir,
+if (virAsprintf(>monpath, "%s/%s", proc->libDir,
"capabilities.monitor.sock") < 0)
goto error;
if (virAsprintf(>monarg, "unix:%s,server,nowait", proc->monpath) < 0)
@@ -8131,7 +8133,7 @@ qemuProcessNew(const char *binary,
* -daemonize we need QEMU to be allowed to create them, rather
* than libvirtd. So we're using libDir which QEMU can write to
*/
-if (virAsprintf(>pidfile, "%s/%s", libDir, "capabilities.pidfile") <
0)
+if (virAsprintf(>pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
goto error;
virPidFileForceCleanupPath(proc->pidfile);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c34ca52ef5..1b8f743861 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -218,6 +218,7 @@ typedef struct _qemuProcess qemuProcess;
typedef qemuProcess *qemuProcessPtr;
struct _qemuProcess {
char *binary;
+char *libDir;
uid_t runUid;
gid_t runGid;
char *qmperr; /* qemu process stderr */
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 08/22] qemu_process: Persist stderr in qemuProcess struct
A qemuProcess struct tracks the entire lifespan of a single QEMU Process
including storing error output when the process terminates or activation
fails.
Error output remains available until qemuProcessFree is called.
The qmperr buffer no longer needs to be maintained outside of the
qemuProcess structure because the structure is used for a single QEMU
process and the structures can be maintained as long as required
to retrieve the process error info.
Capabilities init code is refactored but continues to report QEMU
process error data only when the initial (non KVM) QEMU process activation
fails to result in a usable monitor connection for retrieving
capabilities.
The VIR_ERR_INTERNAL_ERROR "Failed to probe QEMU binary" reporting is
moved into virQEMUCapsInitQMP function and the stderr string is
extracted from the qemuProcess struct using a macro to retrieve the
string.
The same error and log message should be generated, in the same
conditions, after this patch as before.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 27 ---
src/qemu/qemu_process.c | 12
src/qemu/qemu_process.h | 6 --
3 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a957c3bdbd..f5e327097e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4216,8 +4216,7 @@ static int
virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
const char *libDir,
uid_t runUid,
- gid_t runGid,
- char **qmperr)
+ gid_t runGid)
{
qemuProcessPtr proc = NULL;
qemuProcessPtr proc_kvm = NULL;
@@ -4226,7 +4225,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
bool forceTCG = false;
if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
-runUid, runGid, qmperr, forceTCG)))
+runUid, runGid, forceTCG)))
goto cleanup;
if ((rc = qemuProcessRun(proc)) != 0) {
@@ -4242,7 +4241,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
qemuProcessStopQmp(proc);
forceTCG = true;
-proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid,
NULL, forceTCG);
+proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid,
forceTCG);
if ((rc = qemuProcessRun(proc_kvm)) != 0) {
if (rc == 1)
@@ -4257,6 +4256,13 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
ret = 0;
cleanup:
+if (proc && !proc->mon) {
+char *err = QEMU_PROCESS_ERROR(proc) ? QEMU_PROCESS_ERROR(proc) :
_("unknown error");
+
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to probe QEMU binary with QMP: %s"), err);
+}
+
qemuProcessStopQmp(proc);
qemuProcessStopQmp(proc_kvm);
qemuProcessFree(proc);
@@ -4296,7 +4302,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
{
virQEMUCapsPtr qemuCaps;
struct stat sb;
-char *qmperr = NULL;
if (!(qemuCaps = virQEMUCapsNew()))
goto error;
@@ -4323,15 +4328,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
goto error;
}
-if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, ) < 0) {
-virQEMUCapsLogProbeFailure(binary);
-goto error;
-}
-
-if (!qemuCaps->usedQMP) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to probe QEMU binary with QMP: %s"),
- qmperr ? qmperr : _("unknown error"));
+if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0 ||
+!qemuCaps->usedQMP) {
virQEMUCapsLogProbeFailure(binary);
goto error;
}
@@ -4350,7 +4348,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
}
cleanup:
-VIR_FREE(qmperr);
return qemuCaps;
error:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index dda74d5b7a..a741d1cf91 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8091,6 +8091,7 @@ qemuProcessFree(qemuProcessPtr proc)
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
VIR_FREE(proc->pidfile);
+VIR_FREE(proc->qmperr);
VIR_FREE(proc);
}
@@ -8100,7 +8101,6 @@ qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr,
bool forceTCG)
{
qemuProcessPtr proc = NULL;
@@ -8115,8 +8115,6 @@ qemuProcessNew(const char *binary,
proc->runUid = runUid;
proc->runGid = runGid;
-proc->qmperr = qmperr;
-
/* the ".sock" sufix is important to avoid a possible clash with a qemu
* domain called "capabilities"
@@ -8155,8 +8153,7 @@ qemuProcessNew(const char *binary,
* 1 when probing QEMU failed
*/
int
-qemuProcessRun(qemuProcessPtr proc)
-{
+qemuProcessRun(qemuProcessPtr proc){
[libvirt] [PATCH RFC 03/22] qemu_process: Limit qemuProcessNew to const input strings
Prevent compile errors due to trying to use a const string as a
non-const input to qemuProcessNew.
No functionality change.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_process.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index dff0482856..2f9726d463 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8096,7 +8096,7 @@ qemuProcessFree(qemuProcessPtr cmd)
qemuProcessPtr
-qemuProcessNew(char *binary,
+qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 5417cb416f..39a2368ce5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -231,7 +231,7 @@ struct _qemuProcess {
virDomainObjPtr vm;
};
-qemuProcessPtr qemuProcessNew(char *binary,
+qemuProcessPtr qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 04/22] qemu_process: Refer to proc not cmd in process code
s/cmd/proc/ in process code imported from qemu_capabilities.
No functionality is changed. Just variable renaming.
Process code imported from qemu_capabilities was oriented around
starting a process to issue a single QMP command.
Future usecases (ex. baseline, compare) expect to use a single process
to issue multiple different QMP commands.
This patch changes the variable naming from cmd to proc to put focus
on the process being maintained to issue commands.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 14 ++--
src/qemu/qemu_process.c | 140 +--
src/qemu/qemu_process.h | 6 +-
3 files changed, 80 insertions(+), 80 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f6d97648ce..1ea63000e2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4219,7 +4219,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
gid_t runGid,
char **qmperr)
{
-qemuProcessPtr cmd = NULL;
+qemuProcessPtr proc = NULL;
int ret = -1;
int rc;
@@ -4227,31 +4227,31 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
runUid, runGid, qmperr)))
goto cleanup;
-if ((rc = qemuProcessRun(cmd, false)) != 0) {
+if ((rc = qemuProcessRun(proc, false)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
}
-if (virQEMUCapsInitQMPMonitor(qemuCaps, cmd->mon) < 0)
+if (virQEMUCapsInitQMPMonitor(qemuCaps, proc->mon) < 0)
goto cleanup;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
-qemuProcessAbort(cmd);
-if ((rc = qemuProcessRun(cmd, true)) != 0) {
+qemuProcessAbort(proc);
+if ((rc = qemuProcessRun(proc, true)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
}
-if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, cmd->mon) < 0)
+if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon) < 0)
goto cleanup;
}
ret = 0;
cleanup:
-qemuProcessFree(cmd);
+qemuProcessFree(proc);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2f9726d463..e949547124 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8081,17 +8081,17 @@ static qemuMonitorCallbacks callbacks = {
void
-qemuProcessFree(qemuProcessPtr cmd)
+qemuProcessFree(qemuProcessPtr proc)
{
-if (!cmd)
+if (!proc)
return;
-qemuProcessAbort(cmd);
-VIR_FREE(cmd->binary);
-VIR_FREE(cmd->monpath);
-VIR_FREE(cmd->monarg);
-VIR_FREE(cmd->pidfile);
-VIR_FREE(cmd);
+qemuProcessAbort(proc);
+VIR_FREE(proc->binary);
+VIR_FREE(proc->monpath);
+VIR_FREE(proc->monarg);
+VIR_FREE(proc->pidfile);
+VIR_FREE(proc);
}
@@ -8102,25 +8102,25 @@ qemuProcessNew(const char *binary,
gid_t runGid,
char **qmperr)
{
-qemuProcessPtr cmd = NULL;
+qemuProcessPtr proc = NULL;
-if (VIR_ALLOC(cmd) < 0)
+if (VIR_ALLOC(proc) < 0)
goto error;
-if (VIR_STRDUP(cmd->binary, binary) < 0)
+if (VIR_STRDUP(proc->binary, binary) < 0)
goto error;
-cmd->runUid = runUid;
-cmd->runGid = runGid;
-cmd->qmperr = qmperr;
+proc->runUid = runUid;
+proc->runGid = runGid;
+proc->qmperr = qmperr;
/* the ".sock" sufix is important to avoid a possible clash with a qemu
* domain called "capabilities"
*/
-if (virAsprintf(>monpath, "%s/%s", libDir,
+if (virAsprintf(>monpath, "%s/%s", libDir,
"capabilities.monitor.sock") < 0)
goto error;
-if (virAsprintf(>monarg, "unix:%s,server,nowait", cmd->monpath) < 0)
+if (virAsprintf(>monarg, "unix:%s,server,nowait", proc->monpath) < 0)
goto error;
/* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
@@ -8129,19 +8129,19 @@ qemuProcessNew(const char *binary,
* -daemonize we need QEMU to be allowed to create them, rather
* than libvirtd. So we're using libDir which QEMU can write to
*/
-if (virAsprintf(>pidfile, "%s/%s", libDir, "capabilities.pidfile") <
0)
+if (virAsprintf(>pidfile, "%s/%s", libDir, "capabilities.pidfile") <
0)
goto error;
-virPidFileForceCleanupPath(cmd->pidfile);
+virPidFileForceCleanupPath(proc->pidfile);
-cmd->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-cmd->config.data.nix.path = cmd->monpath;
-cmd->config.data.nix.listen = false;
+proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+proc->config.data.nix.path = proc->monpath;
+proc->config.data.nix.listen = false;
-return cmd;
+return proc;
error:
-qemuProcessFree(cmd);
+qemuProcessFree(proc);
return NULL;
}
@@ -8151,7 +8151,7 @@ qemuProcessNew(const char *binary,
* 1 when probing QEMU failed
*/
int
[libvirt] [PATCH RFC 00/22] Move process code to qemu_process
Make process code usable outside qemu_capabilities by moving code
from qemu_capabilities to qemu_process and exposing public functions.
The process code is used to activate a non domain QEMU process for QMP
message exchanges.
This patch set modifies capabilities to use the new public functions.
--
The process code is being decoupled from qemu_capabilities now to
support hypervisor baseline and comparison using QMP commands.
This patch set was originally submitted as part of the baseline patch set:
[libvirt] [PATCH v4 00/37] BaselineHypervisorCPU using QEMU QMP exchanges
https://www.redhat.com/archives/libvir-list/2018-November/msg00091.html
The baseline and comparison requirements are described here:
https://bugzilla.redhat.com/show_bug.cgi?id=1511999
https://bugzilla.redhat.com/show_bug.cgi?id=1511996
I am extracting and resubmitting just the process changes as a stand
alone series to try to make review easier.
The patch set shows capabilities using the public functions.
To see baseline using the public functions...
Look at the "qemu_driver:" patches at the end of
https://www.redhat.com/archives/libvir-list/2018-November/msg00091.html
Also,
The "qemu_driver: Support feature expansion via QEMU when baselining cpu"
patch might be of particular interest because the same QEMU process is
used for both baseline and expansion using QMP commands.
--
Many patches were used to isolate code moves and name changes from other
actual implementation changes.
The patches reuse the pattern of public qemuProcess{Start,Stop} functions
and internal static qemuProcess{Init,Launch,ConnectMonitor} functions
but adds a "Qmp" suffix to make them unique.
A number of patches are about re-partitioning the code into static
functions for initialization, process launch and connection monitor
stuff. This matches the established pattern in qemu_process and seemed
to make sense to do.
For concurrency...
A thread safe library function creates a unique directory under libDir for each
QEMU
process (for QMP messaging) to decouple processes in terms of sockets and
file system footprint.
Every patch should compile independently if applied in sequence.
Chris Venteicher (22):
qemu_process: Move process code from qemu_capabilities to qemu_process
qemu_process: Use qemuProcess prefix
qemu_process: Limit qemuProcessNew to const input strings
qemu_process: Refer to proc not cmd in process code
qemu_process: Use consistent name for stop process function
qemu_capabilities: Stop QEMU process before freeing
qemu_process: Use qemuProcess struct for a single process
qemu_process: Persist stderr in qemuProcess struct
qemu_capabilities: Detect caps probe failure by checking monitor ptr
qemu_process: Introduce qemuProcessStartQmp
qemu_process: Collect monitor code in single function
qemu_process: Store libDir in qemuProcess struct
qemu_process: Setup paths within qemuProcessInitQmp
qemu_process: Stop retaining Qemu Monitor config in qemuProcess
qemu_process: Don't open monitor if process failed
qemu_process: Cleanup qemuProcess alloc function
qemu_process: Cleanup qemuProcessStopQmp function
qemu_process: Catch process free before process stop
qemu_monitor: Make monitor callbacks optional
qemu_process: Enter QMP command mode when starting QEMU Process
qemu_process: Use unique directories for QMP processes
qemu_process: Stop locking QMP process monitor immediately
src/qemu/qemu_capabilities.c | 300 +
src/qemu/qemu_monitor.c | 4 +-
src/qemu/qemu_process.c | 356 +++
src/qemu/qemu_process.h | 37
tests/qemucapabilitiestest.c | 7 +
5 files changed, 444 insertions(+), 260 deletions(-)
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 17/22] qemu_process: Cleanup qemuProcessStopQmp function
qemuProcessStopQmp is one of the 4 public functions used to create and
manage a Qemu process for QMP command exchanges.
Add comment header and debug message.
Other minor code formatting cleanup.
No change in functionality is intended.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 29 ++---
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5ff7d6878c..27a959cf9d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8360,14 +8360,27 @@ qemuProcessStartQmp(qemuProcessPtr proc)
goto cleanup;
}
-
-void
-qemuProcessStopQmp(qemuProcessPtr proc)
+/**
+ * qemuProcessStop:
+ * @proc: Stores Process and Connection State
+ *
+ * Stop Monitor Connection and QEMU Process
+ */
+void qemuProcessStopQmp(qemuProcessPtr proc)
{
-if (proc->mon)
-virObjectUnlock(proc->mon);
-qemuMonitorClose(proc->mon);
-proc->mon = NULL;
+qemuMonitorPtr mon = NULL;
+
+if (!proc)
+return;
+
+VIR_DEBUG("Shutting down proc=%p emulator=%s mon=%p pid=%lld",
+ proc, proc->binary, proc->mon, (long long)proc->pid);
+
+if ((mon = QEMU_PROCESS_MONITOR(proc))) {
+virObjectUnlock(mon);
+qemuMonitorClose(mon);
+proc->mon = NULL;
+}
virCommandAbort(proc->cmd);
virCommandFree(proc->cmd);
@@ -8388,7 +8401,9 @@ qemuProcessStopQmp(qemuProcessPtr proc)
virStrerror(errno, ebuf, sizeof(ebuf)));
}
+
if (proc->pidfile)
unlink(proc->pidfile);
+
proc->pid = 0;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 18/22] qemu_process: Catch process free before process stop
Catch execution paths where qemuProcessFree is called before
qemuProcessStopQmp then report error and force stop before proceeding.
Also added public function header and debug message.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 27a959cf9d..c44e46fc88 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8078,15 +8078,28 @@ static qemuMonitorCallbacks callbacks = {
};
-
-
+/**
+ * qemuProcessFree:
+ * @proc: Stores Process and Connection State
+ *
+ * Free process data structure.
+ */
void
qemuProcessFree(qemuProcessPtr proc)
{
+VIR_DEBUG("proc=%p, proc->mon=%p", proc, (proc ? proc->mon : NULL));
+
if (!proc)
return;
-qemuProcessStopQmp(proc);
+/* This should never be non-NULL if we get here, but just in case... */
+if (proc->mon || proc->pid) {
+VIR_ERROR(_("Unexpected QEMU still active during process free"
+" emulator: %s, pid: %lld, mon: %p"),
+NULLSTR(proc->binary), (long long) proc->pid, proc->mon);
+qemuProcessStopQmp(proc);
+}
+
VIR_FREE(proc->binary);
VIR_FREE(proc->libDir);
VIR_FREE(proc->monpath);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 15/22] qemu_process: Don't open monitor if process failed
Gracefully handle case when proc activation failed prior to calling.
Consistent with the existing code for qemuConnectMonitor (for domains)
in qemu_process.c...
- Handle qemMonitorOpen failure with INFO message and NULL ptr
- Identify parameters passed to qemuMonitorOpen
Monitor callbacks will be removed in future patch so we prep for passing
NULL for the callback pointer.
Set proc->mon to NULL then use VIR_STEAL_PTR if successful to be
consistent with other functions.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d4ed2d6353..55a092ecbb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8236,25 +8236,48 @@ static int
qemuConnectMonitorQmp(qemuProcessPtr proc)
{
int ret = -1;
+qemuMonitorPtr mon = NULL;
+unsigned long long timeout = 0;
+bool retry = true;
+bool enableJson = true;
+virQEMUDriverPtr driver = NULL;
+qemuMonitorCallbacksPtr monCallbacks =
virDomainXMLOptionPtr xmlopt = NULL;
virDomainChrSourceDef monConfig;
VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
proc, NULLSTR(proc->binary), (long long) proc->pid);
+if (!proc || !proc->pid)
+goto ignore;
+
+proc->mon = NULL;
+
monConfig.type = VIR_DOMAIN_CHR_TYPE_UNIX;
monConfig.data.nix.path = proc->monpath;
monConfig.data.nix.listen = false;
+/* Create a NULL Domain object for qemuMonitor */
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
!(proc->vm = virDomainObjNew(xmlopt)))
goto cleanup;
proc->vm->pid = proc->pid;
-if (!(proc->mon = qemuMonitorOpen(proc->vm, , true, true,
- 0, , NULL)))
+mon = qemuMonitorOpen(proc->vm,
+ ,
+ enableJson,
+ retry,
+ timeout,
+ monCallbacks,
+ driver);
+
+if (!mon) {
+VIR_INFO("Failed to connect monitor to emulator %s", proc->binary);
goto ignore;
+}
+
+VIR_STEAL_PTR(proc->mon, mon);
virObjectLock(proc->mon);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 06/22] qemu_capabilities: Stop QEMU process before freeing
Follow the convention established in qemu_process of 1) alloc process structure 2) start process 3) use process 4) stop process 5) free process data structure The process data structure persists after the process activation fails or the process dies or is killed so stderr strings can be retrieved until the process data structure is freed. Signed-off-by: Chris Venteicher --- src/qemu/qemu_capabilities.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 73ec8e5c6e..082874082b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4251,6 +4251,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, ret = 0; cleanup: +qemuProcessStopQmp(proc); qemuProcessFree(proc); return ret; } -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 11/22] qemu_process: Collect monitor code in single function
qemuMonitor code lives in qemuConnectMonitorQmp rather than in
qemuProcessNew and qemuProcessLaunchQmp.
This is consistent with existing structure in qemu_process.c where
qemuConnectMonitor function contains monitor code for domain process
activation.
Simple code moves in this patch. Improvements in later patch.
Only intended functional change in this patch is we don't
move (include) code to initiate process stop on failure to create monitor.
As comments in qemuProcessStartQmp say... Client must always call
qemuProcessStop and qemuProcessFree, even in error cases.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_process.c | 41 -
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b6aa3a9af3..fe4361ed5d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8136,10 +8136,6 @@ qemuProcessNew(const char *binary,
virPidFileForceCleanupPath(proc->pidfile);
-proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-proc->config.data.nix.path = proc->monpath;
-proc->config.data.nix.listen = false;
-
return proc;
error:
@@ -8171,7 +8167,6 @@ qemuProcessInitQmp(qemuProcessPtr proc)
static int
qemuProcessLaunchQmp(qemuProcessPtr proc)
{
-virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
int status = 0;
int ret = -1;
@@ -8223,26 +8218,10 @@ qemuProcessLaunchQmp(qemuProcessPtr proc)
goto ignore;
}
-if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
-!(proc->vm = virDomainObjNew(xmlopt)))
-goto cleanup;
-
-proc->vm->pid = proc->pid;
-
-if (!(proc->mon = qemuMonitorOpen(proc->vm, >config, true, true,
- 0, , NULL)))
-goto ignore;
-
-virObjectLock(proc->mon);
-
ignore:
ret = 0;
cleanup:
-if (!proc->mon)
-qemuProcessStopQmp(proc);
-virObjectUnref(xmlopt);
-
return ret;
}
@@ -8253,13 +8232,33 @@ static int
qemuConnectMonitorQmp(qemuProcessPtr proc)
{
int ret = -1;
+virDomainXMLOptionPtr xmlopt = NULL;
VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
proc, NULLSTR(proc->binary), (long long) proc->pid);
+proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+proc->config.data.nix.path = proc->monpath;
+proc->config.data.nix.listen = false;
+
+if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
+!(proc->vm = virDomainObjNew(xmlopt)))
+goto cleanup;
+
+proc->vm->pid = proc->pid;
+
+if (!(proc->mon = qemuMonitorOpen(proc->vm, >config, true, true,
+ 0, , NULL)))
+goto ignore;
+
+virObjectLock(proc->mon);
+
+ ignore:
ret = 0;
+ cleanup:
VIR_DEBUG("ret=%i", ret);
+virObjectUnref(xmlopt);
return ret;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 10/22] qemu_process: Introduce qemuProcessStartQmp
Move a step closer to the function structure used elsewhere in
qemu_process where qemuProcessStart and qemuProcessStop are the exposed
functions.
qemuProcessStartQmp mirrors qemuProcessStart in calling sub functions to
intialize, launch the process and connect the monitor to the QEMU
process.
static functions qemuProcessInitQmp, qemuProcessLaunchQmp and
qemuConnectMonitorQmp are also introduced.
qemuProcessLaunchQmp is just renamed from qemuProcessRun and
encapsulates all of the original code.
qemuProcessInitQmp and qemuProcessMonitorQmp are introduced as empty
wrappers into which subsequent patches will partition code from
qemuProcessLaunchQmp.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 4 +-
src/qemu/qemu_process.c | 96 +++-
src/qemu/qemu_process.h | 2 +-
3 files changed, 97 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index fbb4336201..7168c470f6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4230,7 +4230,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
goto cleanup;
-if (qemuProcessRun(proc) < 0)
+if (qemuProcessStartQmp(proc) < 0)
goto cleanup;
if (!(mon = QEMU_PROCESS_MONITOR(proc))) {
@@ -4249,7 +4249,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
forceTCG = true;
proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid,
forceTCG);
-if (qemuProcessRun(proc_kvm) < 0)
+if (qemuProcessStartQmp(proc_kvm) < 0)
goto cleanup;
if (!(mon_kvm = QEMU_PROCESS_MONITOR(proc_kvm))) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2640ec2b32..b6aa3a9af3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8148,8 +8148,29 @@ qemuProcessNew(const char *binary,
}
-int
-qemuProcessRun(qemuProcessPtr proc){
+/* Initialize configuration and paths prior to starting QEMU
+ */
+static int
+qemuProcessInitQmp(qemuProcessPtr proc)
+{
+int ret = -1;
+
+VIR_DEBUG("Beginning VM startup process"
+ "emulator=%s",
+ proc->binary);
+
+ret = 0;
+
+VIR_DEBUG("ret=%i", ret);
+return ret;
+}
+
+
+/* Launch QEMU Process
+ */
+static int
+qemuProcessLaunchQmp(qemuProcessPtr proc)
+{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
int status = 0;
@@ -8226,6 +8247,77 @@ qemuProcessRun(qemuProcessPtr proc){
}
+/* Connect Monitor to QEMU Process
+ */
+static int
+qemuConnectMonitorQmp(qemuProcessPtr proc)
+{
+int ret = -1;
+
+VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
+ proc, NULLSTR(proc->binary), (long long) proc->pid);
+
+ret = 0;
+
+VIR_DEBUG("ret=%i", ret);
+return ret;
+}
+
+
+/**
+ * qemuProcessStartQmp:
+ * @proc: Stores Process and Connection State
+ *
+ * Start and connect to QEMU binary so QMP queries can be made.
+ *
+ * Usage:
+ * proc = qemuProcessNew(binary, forceTCG, libDir, runUid, runGid);
+ * qemuProcessStartQmp(proc);
+ * mon = QEMU_PROCESS_MONITOR(proc);
+ * ** Send QMP Queries to QEMU using monitor **
+ * qemuProcessStopQmp(proc);
+ * qemuProcessFree(proc);
+ *
+ * Check monitor is not NULL before using.
+ *
+ * QEMU probing failure results in monitor being NULL but is not considered
+ * an error case and does not result in a negative return value.
+ *
+ * Both qemuProcessStopQmp and qemuProcessFree must be called to cleanup and
+ * free resources, even in activation failure cases.
+ *
+ * Process error output (proc->qmperr) remains available in qemuProcess struct
+ * until qemuProcessFree is called.
+ */
+int
+qemuProcessStartQmp(qemuProcessPtr proc)
+{
+int ret = -1;
+
+VIR_DEBUG("emulator=%s",
+ proc->binary);
+
+if (qemuProcessInitQmp(proc) < 0)
+goto cleanup;
+
+if (qemuProcessLaunchQmp(proc) < 0)
+goto stop;
+
+if (qemuConnectMonitorQmp(proc) < 0)
+goto stop;
+
+ret = 0;
+
+ cleanup:
+VIR_DEBUG("ret=%i", ret);
+return ret;
+
+ stop:
+qemuProcessStopQmp(proc);
+goto cleanup;
+}
+
+
void
qemuProcessStopQmp(qemuProcessPtr proc)
{
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 37194e2501..c34ca52ef5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -246,7 +246,7 @@ qemuProcessPtr qemuProcessNew(const char *binary,
void qemuProcessFree(qemuProcessPtr proc);
-int qemuProcessRun(qemuProcessPtr proc);
+int qemuProcessStartQmp(qemuProcessPtr proc);
void qemuProcessStopQmp(qemuProcessPtr proc);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 05/22] qemu_process: Use consistent name for stop process function
s/qemuProcessAbort/qemuProcessStopQmp/ applied to change function name
used to stop QEMU processes in process code moved from
qemu_capabilities.
No functionality change.
The new name, qemuProcessStopQmp, is consistent with the existing
function qemuProcessStop used to stop Domain processes.
Qmp is added to the end of qemuProcessStop to differentiate between the
Domain and new non-domain version of the functions. qemuProcessStartQmp
will be used in a future patch to mirror the qemuProcessStart function
with a non-domain equivalent.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_process.c | 6 +++---
src/qemu/qemu_process.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1ea63000e2..73ec8e5c6e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4237,7 +4237,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
goto cleanup;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
-qemuProcessAbort(proc);
+qemuProcessStopQmp(proc);
if ((rc = qemuProcessRun(proc, true)) != 0) {
if (rc == 1)
ret = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e949547124..2571024e8e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8086,7 +8086,7 @@ qemuProcessFree(qemuProcessPtr proc)
if (!proc)
return;
-qemuProcessAbort(proc);
+qemuProcessStopQmp(proc);
VIR_FREE(proc->binary);
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
@@ -8222,7 +8222,7 @@ qemuProcessRun(qemuProcessPtr proc,
cleanup:
if (!proc->mon)
-qemuProcessAbort(proc);
+qemuProcessStopQmp(proc);
virObjectUnref(xmlopt);
return ret;
@@ -8234,7 +8234,7 @@ qemuProcessRun(qemuProcessPtr proc,
void
-qemuProcessAbort(qemuProcessPtr proc)
+qemuProcessStopQmp(qemuProcessPtr proc)
{
if (proc->mon)
virObjectUnlock(proc->mon);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 161311d007..25343c4592 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -241,6 +241,6 @@ void qemuProcessFree(qemuProcessPtr proc);
int qemuProcessRun(qemuProcessPtr proc, bool forceTCG);
-void qemuProcessAbort(qemuProcessPtr proc);
+void qemuProcessStopQmp(qemuProcessPtr proc);
#endif /* __QEMU_PROCESS_H__ */
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 09/22] qemu_capabilities: Detect caps probe failure by checking monitor ptr
Failure to connect to QEMU to probe capabilities is not considered a error case
and does not result in a negative return value.
Check for a NULL monitor connection pointer before trying to send capabilities
commands to QEMU rather than depending on a special positive return value.
This simplifies logic and is more consistent with the operation of existing
qemu_process functions.
A macro is introduced to easily obtain the monitor pointer from the
qemuProcess structure.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 28 ++--
src/qemu/qemu_process.c | 9 +
src/qemu/qemu_process.h | 3 +++
3 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f5e327097e..fbb4336201 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4220,43 +4220,51 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
{
qemuProcessPtr proc = NULL;
qemuProcessPtr proc_kvm = NULL;
+qemuMonitorPtr mon = NULL;
+qemuMonitorPtr mon_kvm = NULL;
int ret = -1;
-int rc;
bool forceTCG = false;
if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
runUid, runGid, forceTCG)))
goto cleanup;
-if ((rc = qemuProcessRun(proc)) != 0) {
-if (rc == 1)
-ret = 0;
+
+if (qemuProcessRun(proc) < 0)
+goto cleanup;
+
+if (!(mon = QEMU_PROCESS_MONITOR(proc))) {
+ret = 0; /* Failure probing QEMU not considered error case */
goto cleanup;
}
-if (virQEMUCapsInitQMPMonitor(qemuCaps, proc->mon) < 0)
+/* Pull capabilities from QEMU */
+if (virQEMUCapsInitQMPMonitor(qemuCaps, mon) < 0)
goto cleanup;
+/* Pull capabilities again if KVM supported */
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
qemuProcessStopQmp(proc);
forceTCG = true;
proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid,
forceTCG);
-if ((rc = qemuProcessRun(proc_kvm)) != 0) {
-if (rc == 1)
-ret = 0;
+if (qemuProcessRun(proc_kvm) < 0)
+goto cleanup;
+
+if (!(mon_kvm = QEMU_PROCESS_MONITOR(proc_kvm))) {
+ret = 0; /* Failure probing QEMU not considered error case */
goto cleanup;
}
-if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc_kvm->mon) < 0)
+if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, mon_kvm) < 0)
goto cleanup;
}
ret = 0;
cleanup:
-if (proc && !proc->mon) {
+if (!mon) {
char *err = QEMU_PROCESS_ERROR(proc) ? QEMU_PROCESS_ERROR(proc) :
_("unknown error");
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a741d1cf91..2640ec2b32 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8148,10 +8148,6 @@ qemuProcessNew(const char *binary,
}
-/* Returns -1 on fatal error,
- * 0 on success,
- * 1 when probing QEMU failed
- */
int
qemuProcessRun(qemuProcessPtr proc){
virDomainXMLOptionPtr xmlopt = NULL;
@@ -8218,6 +8214,7 @@ qemuProcessRun(qemuProcessPtr proc){
virObjectLock(proc->mon);
+ ignore:
ret = 0;
cleanup:
@@ -8226,10 +8223,6 @@ qemuProcessRun(qemuProcessPtr proc){
virObjectUnref(xmlopt);
return ret;
-
- ignore:
-ret = 1;
-goto cleanup;
}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index abd80baf5c..37194e2501 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -235,6 +235,9 @@ struct _qemuProcess {
#define QEMU_PROCESS_ERROR(proc) \
((char *) (proc ? proc->qmperr: NULL))
+#define QEMU_PROCESS_MONITOR(proc) \
+ ((qemuMonitorPtr) (proc ? proc->mon : NULL))
+
qemuProcessPtr qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 02/22] qemu_process: Use qemuProcess prefix
s/virQEMUCapsInitQMPCommand/qemuProcess/
No functionality change.
Use appropriate prefix in moved code.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 14 +++---
src/qemu/qemu_process.c | 28 ++--
src/qemu/qemu_process.h | 22 +++---
3 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0f70fdf46d..f6d97648ce 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4219,15 +4219,15 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
gid_t runGid,
char **qmperr)
{
-virQEMUCapsInitQMPCommandPtr cmd = NULL;
+qemuProcessPtr cmd = NULL;
int ret = -1;
int rc;
-if (!(cmd = virQEMUCapsInitQMPCommandNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr)))
+if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
+runUid, runGid, qmperr)))
goto cleanup;
-if ((rc = virQEMUCapsInitQMPCommandRun(cmd, false)) != 0) {
+if ((rc = qemuProcessRun(cmd, false)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
@@ -4237,8 +4237,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
goto cleanup;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
-virQEMUCapsInitQMPCommandAbort(cmd);
-if ((rc = virQEMUCapsInitQMPCommandRun(cmd, true)) != 0) {
+qemuProcessAbort(cmd);
+if ((rc = qemuProcessRun(cmd, true)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
@@ -4251,7 +4251,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
ret = 0;
cleanup:
-virQEMUCapsInitQMPCommandFree(cmd);
+qemuProcessFree(cmd);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0b3922fa39..dff0482856 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8081,12 +8081,12 @@ static qemuMonitorCallbacks callbacks = {
void
-virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
+qemuProcessFree(qemuProcessPtr cmd)
{
if (!cmd)
return;
-virQEMUCapsInitQMPCommandAbort(cmd);
+qemuProcessAbort(cmd);
VIR_FREE(cmd->binary);
VIR_FREE(cmd->monpath);
VIR_FREE(cmd->monarg);
@@ -8095,14 +8095,14 @@
virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
}
-virQEMUCapsInitQMPCommandPtr
-virQEMUCapsInitQMPCommandNew(char *binary,
- const char *libDir,
- uid_t runUid,
- gid_t runGid,
- char **qmperr)
+qemuProcessPtr
+qemuProcessNew(char *binary,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ char **qmperr)
{
-virQEMUCapsInitQMPCommandPtr cmd = NULL;
+qemuProcessPtr cmd = NULL;
if (VIR_ALLOC(cmd) < 0)
goto error;
@@ -8141,7 +8141,7 @@ virQEMUCapsInitQMPCommandNew(char *binary,
return cmd;
error:
-virQEMUCapsInitQMPCommandFree(cmd);
+qemuProcessFree(cmd);
return NULL;
}
@@ -8151,8 +8151,8 @@ virQEMUCapsInitQMPCommandNew(char *binary,
* 1 when probing QEMU failed
*/
int
-virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
- bool forceTCG)
+qemuProcessRun(qemuProcessPtr cmd,
+ bool forceTCG)
{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
@@ -8222,7 +8222,7 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr
cmd,
cleanup:
if (!cmd->mon)
-virQEMUCapsInitQMPCommandAbort(cmd);
+qemuProcessAbort(cmd);
virObjectUnref(xmlopt);
return ret;
@@ -8234,7 +8234,7 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr
cmd,
void
-virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
+qemuProcessAbort(qemuProcessPtr cmd)
{
if (cmd->mon)
virObjectUnlock(cmd->mon);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 4ba3988e3d..5417cb416f 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -214,9 +214,9 @@ int qemuProcessStartManagedPRDaemon(virDomainObjPtr vm);
void qemuProcessKillManagedPRDaemon(virDomainObjPtr vm);
-typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
-typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
-struct _virQEMUCapsInitQMPCommand {
+typedef struct _qemuProcess qemuProcess;
+typedef qemuProcess *qemuProcessPtr;
+struct _qemuProcess {
char *binary;
uid_t runUid;
gid_t runGid;
@@ -231,16 +231,16 @@ struct _virQEMUCapsInitQMPCommand {
virDomainObjPtr vm;
};
-virQEMUCapsInitQMPCommandPtr virQEMUCapsInitQMPCommandNew(char *binary,
- const char *libDir,
-
[libvirt] [PATCH RFC 01/22] qemu_process: Move process code from qemu_capabilities to qemu_process
Qemu process code in qemu_capabilities.c is moved to qemu_process.c in
order to make the code usable outside the original capabilities
usecases.
This process code activates and manages Qemu processes without
establishing a guest domain.
This patch is a straight cut/paste move between files.
Following patches modify the process code
making it more generic and consistent with qemu_process.
Signed-off-by: Chris Venteicher
---
src/qemu/qemu_capabilities.c | 218 +--
src/qemu/qemu_process.c | 201
src/qemu/qemu_process.h | 29 +
3 files changed, 231 insertions(+), 217 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ca5af3297..0f70fdf46d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -47,6 +47,7 @@
#define __QEMU_CAPSPRIV_H_ALLOW__
#include "qemu_capspriv.h"
#include "qemu_qapi.h"
+#include "qemu_process.h"
#include
#include
@@ -3917,18 +3918,6 @@ virQEMUCapsIsValid(void *data,
}
-static void virQEMUCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
- virDomainObjPtr vm ATTRIBUTE_UNUSED,
- void *opaque ATTRIBUTE_UNUSED)
-{
-}
-
-static qemuMonitorCallbacks callbacks = {
-.eofNotify = virQEMUCapsMonitorNotify,
-.errorNotify = virQEMUCapsMonitorNotify,
-};
-
-
/**
* virQEMUCapsInitQMPArch:
* @qemuCaps: QEMU capabilities
@@ -4223,211 +4212,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
}
-typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
-typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
-struct _virQEMUCapsInitQMPCommand {
-char *binary;
-uid_t runUid;
-gid_t runGid;
-char **qmperr;
-char *monarg;
-char *monpath;
-char *pidfile;
-virCommandPtr cmd;
-qemuMonitorPtr mon;
-virDomainChrSourceDef config;
-pid_t pid;
-virDomainObjPtr vm;
-};
-
-
-static void
-virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
-{
-if (cmd->mon)
-virObjectUnlock(cmd->mon);
-qemuMonitorClose(cmd->mon);
-cmd->mon = NULL;
-
-virCommandAbort(cmd->cmd);
-virCommandFree(cmd->cmd);
-cmd->cmd = NULL;
-
-if (cmd->monpath)
-unlink(cmd->monpath);
-
-virDomainObjEndAPI(>vm);
-
-if (cmd->pid != 0) {
-char ebuf[1024];
-
-VIR_DEBUG("Killing QMP caps process %lld", (long long)cmd->pid);
-if (virProcessKill(cmd->pid, SIGKILL) < 0 && errno != ESRCH)
-VIR_ERROR(_("Failed to kill process %lld: %s"),
- (long long)cmd->pid,
- virStrerror(errno, ebuf, sizeof(ebuf)));
-
-VIR_FREE(*cmd->qmperr);
-}
-if (cmd->pidfile)
-unlink(cmd->pidfile);
-cmd->pid = 0;
-}
-
-
-static void
-virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
-{
-if (!cmd)
-return;
-
-virQEMUCapsInitQMPCommandAbort(cmd);
-VIR_FREE(cmd->binary);
-VIR_FREE(cmd->monpath);
-VIR_FREE(cmd->monarg);
-VIR_FREE(cmd->pidfile);
-VIR_FREE(cmd);
-}
-
-
-static virQEMUCapsInitQMPCommandPtr
-virQEMUCapsInitQMPCommandNew(char *binary,
- const char *libDir,
- uid_t runUid,
- gid_t runGid,
- char **qmperr)
-{
-virQEMUCapsInitQMPCommandPtr cmd = NULL;
-
-if (VIR_ALLOC(cmd) < 0)
-goto error;
-
-if (VIR_STRDUP(cmd->binary, binary) < 0)
-goto error;
-
-cmd->runUid = runUid;
-cmd->runGid = runGid;
-cmd->qmperr = qmperr;
-
-/* the ".sock" sufix is important to avoid a possible clash with a qemu
- * domain called "capabilities"
- */
-if (virAsprintf(>monpath, "%s/%s", libDir,
-"capabilities.monitor.sock") < 0)
-goto error;
-if (virAsprintf(>monarg, "unix:%s,server,nowait", cmd->monpath) < 0)
-goto error;
-
-/* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
- * with a qemu domain called "capabilities"
- * Normally we'd use runDir for pid files, but because we're using
- * -daemonize we need QEMU to be allowed to create them, rather
- * than libvirtd. So we're using libDir which QEMU can write to
- */
-if (virAsprintf(>pidfile, "%s/%s", libDir, "capabilities.pidfile") <
0)
-goto error;
-
-virPidFileForceCleanupPath(cmd->pidfile);
-
-cmd->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-cmd->config.data.nix.path = cmd->monpath;
-cmd->config.data.nix.listen = false;
-
-return cmd;
-
- error:
-virQEMUCapsInitQMPCommandFree(cmd);
-return NULL;
-}
-
-
-/* Returns -1 on fatal error,
- * 0 on success,
- * 1 when probing QEMU failed
- */
-static int
-virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
-
Re: [libvirt] [PATCH v3] lxc: Include support to lxc version 3.0 or higher.
Em sáb, 10 de nov de 2018 às 11:17, John Ferlan escreveu:
>
>
>
> On 11/9/18 12:30 PM, Julio Faracco wrote:
> > This patch introduce the new settings for LXC 3.0 or higher. The older
> > versions keep the compatibility to deprecated settings for LXC, but
> > after release 3.0, the compatibility was removed. This commit adds the
> > support to the refactored settings.
> >
> > v1-v2: Michal's suggestions to handle differences between versions.
> > v2-v3: Adding suggestions from Pino and John too.
>
> These type of comments would go below the --- below so that they're not
> part of commit history...
>
> >
> > Signed-off-by: Julio Faracco
> > ---
> > src/lxc/lxc_native.c | 45 +++-
> > 1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> > index cbdec723dd..d3ba12bb0e 100644
> > --- a/src/lxc/lxc_native.c
> > +++ b/src/lxc/lxc_native.c
>
> [...]
>
> > @@ -724,12 +734,13 @@ lxcIdmapWalkCallback(const char *name,
> > virConfValuePtr value, void *data)
> > char type;
> > unsigned long start, target, count;
> >
> > -if (STRNEQ(name, "lxc.id_map") || !value->str)
> > +/* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */
> > +if (STRNEQ(name, "lxc.idmap") || STRNEQ(name, "lxc.id_map") ||
> > !value->str)
> > return 0;
>
> This one caused lxcconf2xmltest to fail and needs to change to:
>
> /* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */
> if (STRNEQ(name, "lxc.idmap") || !value->str) {
> if (!value->str || STRNEQ(name, "lxc.id_map"))
> return 0;
> }
>
> The failure occurred because of the STRNEQ OR not being true (silly me
> on first pass not running the tests too ;-))
>
> >
> > if (sscanf(value->str, "%c %lu %lu %lu", ,
> > , , ) != 4) {
> > -virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid lxc.id_map:
> > '%s'"),
> > +virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid: '%s'"),
>
> Do you mind if I alter this to:
>
> virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid %s: '%s'"),
>name, value->str);
>
> That way the conf name string is provided like it was before
>
>
> > value->str);
> > return -1;
> > }
> > @@ -1041,19 +1052,27 @@ lxcParseConfigString(const char *config,
> > if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0)
> > goto error;
> >
> > -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 ||
> > -VIR_STRDUP(vmdef->name, value) < 0)
> > -goto error;
> > +if (virConfGetValueString(properties, "lxc.uts.name", ) <= 0) {
> > +virResetLastError();
> > +
> > +/* Check for pre LXC 3.0 legacy key */
> > +if (virConfGetValueString(properties, "lxc.utsname", ) <= 0)
> > +goto error;
> > +}
> > +
>
> I think in this case the @value needs to be restored... Previous if the
> GetValueString OR the VIR_STRDUP of the value was < 0, we go to error.
> Although I'm not quite sure how @value would be NULL so as to cause the
> subsequent line to be executed... In any case, copying @value needs to
> be done, so add:
>
> if (VIR_STRDUP(vmdef->name, value) < 0)
> goto error;
>
> Which I can add if you agree.
No problems, John. You can go ahead with the changes.
I forgot too add VIR_STRDUP after checking the property.
It was causing the test error.
>
> With those changes,
>
> Reviewed-by: John Ferlan
>
> John
>
> As a follow-up maybe adding/altering/updating the tests/lxcconf2xmldata
> to include both pre and post 3.0 type data would be a good thing.
Yes, I agree too. But only config files that don't have netowork settings.
Version 3.X and higher have another syntax to configure network too.
And it was not implemented yet. I'm planning to propose this feature
in the future.
>
> [...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
