7:08 a.m.
Hi, all.
This patch series aims to handle case when hostdev usb is detached and then
attached back on node. Currently libvirtd does not track these events so that in
the result hostdev usb is not usable.
On the first glace it make sense to delete device from qemu when hostdev is
detached on node and then add device back when hostdev is attached back. But
these series adds a different behaviour. On detaching device is deleted from
qemu and dummy (missing hostaddr and hostbus values) hostdev is inserted, then
on attaching device on node dummy device is deleted and proper one is inserted.
This way we have the same semantics of 'missing' flag for hostdev as in case of
hostdev missing on startup/migration to different host. Also with such
approach we additionally handle cases when hostdev is missing on startup but
later is attached to node.
Known issues:
The last patch tries to handle case when hostdev attached/detached when
libvirtd is down but if fails in one scenario. If hostdev is detached and thus
dummy device in inserted in qemu then if libvirtd is down and hostdev is
attached to node then qemu tries to bind appeared hostdev to the dummy device
but fails as device is missing in qemu's mount namespace. Later when libvirtd
is up and tries to replace dummy device with actual one the device_add fails
because libusb the qemu uses tracks that deviced was missed ealier. For this
reason I proposed to get rid of dummy device in RFC [1]. Alternatively libusb
can be fixed.
[1] https://www.redhat.com/archives/libvir-list/2019-August/msg01396.html
Nikolay Shirokovskiy (6):
qemu: track hostdev delete cause
qemu: handle usb hostdev add/del udev events
qemu: handle usb hostdev add/del on device DELETED_EVENT
conf: parse hostdev missing flag
qemu: handle libvirtd restarts during hostdev reattaching
qemu: handle hostdev add/del when libvirtd is down
src/conf/domain_conf.c | 32 ++++
src/conf/domain_conf.h | 10 ++
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_domain.c | 2 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 376 +++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_hotplug.c | 94 ++++++++--
src/qemu/qemu_hotplug.h | 7 +-
src/qemu/qemu_process.c | 55 +++++-
src/util/virhostdev.c | 6 +-
tests/qemuhotplugtest.c | 2 +-
12 files changed, 558 insertions(+), 33 deletions(-)
--
2.23.0
7:08 a.m.
New subject: [libvirt] [PATCH 1/6] qemu: track hostdev delete cause
qemuDomainDetachDeviceLive is called when client asks for detaching
a device. We are going to call this function when usb device is
detached on node itself. The function itself requests qemu to detach
device from guest but as device detaching is asynchronous libvirt
config changes are done in DEVICE_DELETED event handler. As we
want to keep device in libvirt config if device detached from node
opposite to the case of client initiated detaching we need to
track the delete cause.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/conf/domain_conf.h | 9 +++++++++
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_hotplug.c | 21 ++++++++++++++++++++-
src/qemu/qemu_hotplug.h | 3 ++-
tests/qemuhotplugtest.c | 2 +-
5 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 33cef5b75c..49392d0286 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -326,6 +326,14 @@ struct _virDomainHostdevCaps {
} u;
};
+typedef enum {
+ VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_NONE = 0,
+ VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_CLIENT,
+ VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING,
+
+ VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_LAST
+} virDomainHostdevDeleteCauseType;
+
/* basic device for direct passthrough */
struct _virDomainHostdevDef {
@@ -343,6 +351,7 @@ struct _virDomainHostdevDef {
bool missing;
bool readonly;
bool shareable;
+ virDomainHostdevDeleteCauseType deleteCause;
union {
virDomainHostdevSubsys subsys;
virDomainHostdevCaps caps;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 78f5471b79..2378a2e7d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9123,7 +9123,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
int rc;
- if ((rc = qemuDomainDetachDeviceLive(vm, dev_copy, driver, false)) < 0)
+ if ((rc = qemuDomainDetachDeviceLive(vm, dev_copy, driver, false, false)) <
0)
goto cleanup;
if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm,
QEMU_ASYNC_JOB_NONE) < 0)
@@ -9212,7 +9212,7 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver,
if (virDomainDefFindDevice(def, alias, &dev, true) < 0)
goto cleanup;
- if ((rc = qemuDomainDetachDeviceLive(vm, &dev, driver, true)) < 0)
+ if ((rc = qemuDomainDetachDeviceLive(vm, &dev, driver, true, false)) < 0)
goto cleanup;
if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm,
QEMU_ASYNC_JOB_NONE) < 0)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 63acb9c451..08e60dcd0e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5721,7 +5721,8 @@ int
qemuDomainDetachDeviceLive(virDomainObjPtr vm,
virDomainDeviceDefPtr match,
virQEMUDriverPtr driver,
- bool async)
+ bool async,
+ bool reattaching)
{
virDomainDeviceDef detach = { .type = match->type };
virDomainDeviceInfoPtr info = NULL;
@@ -5880,6 +5881,24 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
goto cleanup;
}
+ if (detach.type == VIR_DOMAIN_DEVICE_HOSTDEV) {
+ virDomainHostdevDefPtr hostdev = detach.data.hostdev;
+
+ /*
+ * Why having additional check in second branch? Suppose client
+ * asks for device detaching and we pass the intention to qemu
+ * but don't get DEVICE_DELETED event yet. Next USB is detached
+ * on node and we have this function called again. If we reset
+ * delete cause to 'reattaching' then device will be left in
+ * libvirt config after handling DEVICE_DELETED event while
+ * it should not as client asked to detach the device before.
+ */
+ if (!reattaching)
+ hostdev->deleteCause = VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_CLIENT;
+ else if (hostdev->deleteCause != VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_CLIENT)
+ hostdev->deleteCause = VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING;
+ }
+
if (async) {
ret = 0;
} else {
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 896e6c7b98..bf812eab1a 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -116,7 +116,8 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
int qemuDomainDetachDeviceLive(virDomainObjPtr vm,
virDomainDeviceDefPtr match,
virQEMUDriverPtr driver,
- bool async);
+ bool async,
+ bool reattaching);
void qemuDomainRemoveVcpuAlias(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index b6aad330a9..ef91d4f131 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -151,7 +151,7 @@ testQemuHotplugDetach(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_CHR:
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_WATCHDOG:
- ret = qemuDomainDetachDeviceLive(vm, dev, &driver, async);
+ ret = qemuDomainDetachDeviceLive(vm, dev, &driver, async, false);
break;
default:
VIR_TEST_VERBOSE("device type '%s' cannot be detached",
--
2.23.0
7:08 a.m.
New subject: [libvirt] [PATCH 2/6] qemu: handle usb hostdev add/del udev events
In either case detach qemu hostdev device (it is dummy in case
node hostdev is absent). Meaningful processing will be in DEVICE_DELETED
event handler.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_domain.c | 2 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 344 ++++++++++++++++++++++++++++++++++++++-
5 files changed, 352 insertions(+), 1 deletion(-)
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index d16b315ebc..8be0dee396 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -85,6 +85,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
-I$(srcdir)/conf \
-I$(srcdir)/secret \
$(AM_CFLAGS) \
+ $(UDEV_CFLAGS) \
$(NULL)
libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS)
libvirt_driver_qemu_impl_la_LIBADD = \
@@ -93,6 +94,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \
$(LIBNL_LIBS) \
$(SELINUX_LIBS) \
$(LIBXML_LIBS) \
+ $(UDEV_LIBS) \
$(NULL)
libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 0cbddd7a9c..2e50bb0950 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -294,6 +294,9 @@ struct _virQEMUDriver {
/* Immutable pointer, self-locking APIs */
virHashAtomicPtr migrationErrors;
+
+ struct udev_monitor *udev_monitor;
+ int udev_watch;
};
virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 657f3ecfe4..4784804d1e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -15034,6 +15034,8 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
case QEMU_PROCESS_EVENT_BLOCK_JOB:
case QEMU_PROCESS_EVENT_MONITOR_EOF:
+ case QEMU_PROCESS_EVENT_USB_REMOVED:
+ case QEMU_PROCESS_EVENT_USB_ADDED:
VIR_FREE(event->data);
break;
case QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d097f23342..94aea62693 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -521,6 +521,8 @@ typedef enum {
QEMU_PROCESS_EVENT_MONITOR_EOF,
QEMU_PROCESS_EVENT_PR_DISCONNECT,
QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED,
+ QEMU_PROCESS_EVENT_USB_REMOVED,
+ QEMU_PROCESS_EVENT_USB_ADDED,
QEMU_PROCESS_EVENT_LAST
} qemuProcessEventType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2378a2e7d0..33b75a3c71 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -34,6 +34,7 @@
#include <sys/ioctl.h>
#include <sys/un.h>
#include <byteswap.h>
+#include <libudev.h>
#include "qemu_driver.h"
@@ -719,6 +720,254 @@ qemuDomainFindMaxID(virDomainObjPtr vm,
}
+struct qemuUdevUSBRemoveData {
+ unsigned int bus;
+ unsigned int device;
+};
+
+struct qemuUdevUSBAddData {
+ unsigned int vendor;
+ unsigned int product;
+};
+
+struct qemuUdevUSBEventData {
+ union {
+ struct qemuUdevUSBRemoveData remove;
+ struct qemuUdevUSBAddData add;
+ } data;
+ bool found;
+ bool remove;
+};
+
+static int
+qemuUdevUSBHandleEvent(virDomainObjPtr vm, void *opaque)
+{
+ struct qemuUdevUSBEventData *data = opaque;
+ struct qemuProcessEvent *event = NULL;
+ size_t i;
+
+ if (data->found)
+ return 0;
+
+ virObjectLock(vm);
+
+ if (!virDomainObjIsActive(vm))
+ goto cleanup;
+
+ for (i = 0; i < vm->def->nhostdevs; i++) {
+ virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
+ virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
+
+ if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+ continue;
+
+ if (data->remove) {
+ if (usbsrc->bus != data->data.remove.bus ||
+ usbsrc->device != data->data.remove.device)
+ continue;
+ } else {
+ if (usbsrc->vendor != data->data.add.vendor ||
+ usbsrc->product != data->data.add.product)
+ continue;
+ }
+
+ data->found = true;
+
+ if (VIR_ALLOC(event) < 0)
+ goto cleanup;
+
+ if (data->remove) {
+ struct qemuUdevUSBRemoveData *rm_data;
+
+
+ if (VIR_ALLOC(rm_data) < 0)
+ goto cleanup;
+
+ *rm_data = data->data.remove;
+ event->data = rm_data;
+ event->eventType = QEMU_PROCESS_EVENT_USB_REMOVED;
+ } else {
+ struct qemuUdevUSBAddData *add_data;
+
+ if (VIR_ALLOC(add_data) < 0)
+ goto cleanup;
+
+ *add_data = data->data.add;
+ event->data = add_data;
+ event->eventType = QEMU_PROCESS_EVENT_USB_ADDED;
+ }
+
+ event->vm = virObjectRef(vm);
+
+ if (virThreadPoolSendJob(qemu_driver->workerPool, 0, event) < 0) {
+ virObjectUnref(vm);
+ goto cleanup;
+ }
+
+ event = NULL;
+
+ break;
+ }
+
+ cleanup:
+ virObjectUnlock(vm);
+
+ qemuProcessEventFree(event);
+
+ return 0;
+}
+
+
+static void
+qemuUdevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
+ int fd ATTRIBUTE_UNUSED,
+ int events ATTRIBUTE_UNUSED,
+ void *data ATTRIBUTE_UNUSED)
+{
+ struct qemuUdevUSBEventData event_data;
+ struct udev_device *dev = NULL;
+ const char *action;
+ const char *devtype;
+ const char *tmp;
+
+ /* libvirtd daemon do not run event loop before full state drivers
+ * initialization. Also state drivers uninitialized only after
+ * full stop of event loop. In short driver initialization/uninitialization
+ * and handling events occurs in same main loop thread. Thus we
+ * don't need any locking here. */
+
+ if (!(dev = udev_monitor_receive_device(qemu_driver->udev_monitor))) {
+ VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
+ if (errno == EAGAIN || errno == EWOULDBLOCK) {
+ VIR_WARNINGS_RESET
+ return;
+ }
+
+ virReportSystemError(errno, "%s",
+ _("failed to receive device from udev monitor"));
+ return;
+ }
+
+ devtype = udev_device_get_devtype(dev);
+
+ if (STRNEQ_NULLABLE(devtype, "usb_device"))
+ goto cleanup;
+
+ if (!(action = udev_device_get_action(dev))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to receive action from udev monitor"));
+ goto cleanup;
+ }
+
+ if (STREQ(action, "remove")) {
+ struct qemuUdevUSBRemoveData *rm_data = &event_data.data.remove;
+
+ if (!(tmp = udev_device_get_property_value(dev, "BUSNUM"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to receive busnum from udev monitor"));
+ goto cleanup;
+ }
+ if (virStrToLong_ui(tmp, NULL, 10, &rm_data->bus) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to convert busnum to int"));
+ goto cleanup;
+ }
+
+ if (!(tmp = udev_device_get_property_value(dev, "DEVNUM"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to receive devnum from udev monitor"));
+ goto cleanup;
+ }
+ if (virStrToLong_ui(tmp, NULL, 10, &rm_data->device) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to convert devnum to int"));
+ goto cleanup;
+ }
+ event_data.remove = true;
+ } else if (STREQ(action, "add")) {
+ struct qemuUdevUSBAddData *add_data = &event_data.data.add;
+
+ if (!(tmp = udev_device_get_property_value(dev, "ID_VENDOR_ID"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to receive vendor from udev monitor"));
+ goto cleanup;
+ }
+ if (virStrToLong_ui(tmp, NULL, 16, &add_data->vendor) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to convert vendor to int"));
+ goto cleanup;
+ }
+
+ if (!(tmp = udev_device_get_property_value(dev, "ID_MODEL_ID"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to receive product from udev monitor"));
+ goto cleanup;
+ }
+ if (virStrToLong_ui(tmp, NULL, 16, &add_data->product) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to convert product to int"));
+ goto cleanup;
+ }
+ event_data.remove = false;
+ }
+
+ event_data.found = false;
+ virDomainObjListForEach(qemu_driver->domains, qemuUdevUSBHandleEvent,
&event_data);
+
+ cleanup:
+ udev_device_unref(dev);
+}
+
+
+static int
+qemuUdevInitialize(void)
+{
+ struct udev *udev;
+
+ if (!(udev = udev_new())) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to create udev context"));
+ return -1;
+ }
+
+ if (!(qemu_driver->udev_monitor = udev_monitor_new_from_netlink(udev,
"udev"))) {
+ udev_unref(udev);
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("udev_monitor_new_from_netlink returned NULL"));
+ return -1;
+ }
+
+ udev_monitor_enable_receiving(qemu_driver->udev_monitor);
+
+ qemu_driver->udev_watch =
virEventAddHandle(udev_monitor_get_fd(qemu_driver->udev_monitor),
+ VIR_EVENT_HANDLE_READABLE,
+ qemuUdevEventHandleCallback, NULL,
NULL);
+
+ if (qemu_driver->udev_watch < 0)
+ return -1;
+
+ return 0;
+}
+
+
+static void
+qemuUdevCleanup(void)
+{
+ if (qemu_driver->udev_monitor) {
+ struct udev *udev = udev_monitor_get_udev(qemu_driver->udev_monitor);
+
+ udev_monitor_unref(qemu_driver->udev_monitor);
+ udev_unref(udev);
+ qemu_driver->udev_monitor = NULL;
+ }
+
+ if (qemu_driver->udev_watch > 0) {
+ virEventRemoveHandle(qemu_driver->udev_watch);
+ qemu_driver->udev_watch = 0;
+ }
+}
+
+
/**
* qemuStateInitialize:
*
@@ -1030,6 +1279,9 @@ qemuStateInitialize(bool privileged,
if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew()))
goto error;
+ if (qemuUdevInitialize() < 0)
+ goto error;
+
/* Get all the running persistent or transient configs first */
if (virDomainObjListLoadAllConfigs(qemu_driver->domains,
cfg->stateDir,
@@ -1239,6 +1491,8 @@ qemuStateCleanup(void)
virLockManagerPluginUnref(qemu_driver->lockManager);
+ qemuUdevCleanup();
+
virMutexDestroy(&qemu_driver->lock);
VIR_FREE(qemu_driver);
@@ -5011,7 +5265,89 @@ processRdmaGidStatusChangedEvent(virDomainObjPtr vm,
}
-static void qemuProcessEventHandler(void *data, void *opaque)
+static void
+processUSBAddedEvent(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ struct qemuUdevUSBAddData *data)
+{
+ virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+ virDomainHostdevDefPtr hostdev;
+ size_t i;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ return;
+
+ if (!virDomainObjIsActive(vm)) {
+ VIR_DEBUG("Domain is not running");
+ goto cleanup;
+ }
+
+ for (i = 0; i < vm->def->nhostdevs; i++) {
+ virDomainHostdevSubsysUSBPtr usbsrc;
+
+ hostdev = vm->def->hostdevs[i];
+ usbsrc = &hostdev->source.subsys.u.usb;
+
+ if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB
&&
+ usbsrc->vendor == data->vendor && usbsrc->product ==
data->product &&
+ hostdev->missing)
+ break;
+ }
+
+ if (i == vm->def->nhostdevs)
+ goto cleanup;
+
+ dev.data.hostdev = hostdev;
+ if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
+ goto cleanup;
+
+ cleanup:
+ qemuDomainObjEndJob(driver, vm);
+}
+
+
+static void
+processUSBRemovedEvent(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ struct qemuUdevUSBRemoveData *data)
+{
+ size_t i;
+ virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+ virDomainHostdevDefPtr hostdev;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ return;
+
+ if (!virDomainObjIsActive(vm)) {
+ VIR_DEBUG("Domain is not running");
+ goto cleanup;
+ }
+
+ for (i = 0; i < vm->def->nhostdevs; i++) {
+ virDomainHostdevSubsysUSBPtr usbsrc;
+
+ hostdev = vm->def->hostdevs[i];
+ usbsrc = &hostdev->source.subsys.u.usb;
+
+ if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB
&&
+ usbsrc->bus == data->bus && usbsrc->device ==
data->device)
+ break;
+ }
+
+ if (i == vm->def->nhostdevs)
+ goto cleanup;
+
+ dev.data.hostdev = hostdev;
+ if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
+ goto cleanup;
+
+ cleanup:
+ qemuDomainObjEndJob(driver, vm);
+}
+
+
+static void
+qemuProcessEventHandler(void *data, void *opaque)
{
struct qemuProcessEvent *processEvent = data;
virDomainObjPtr vm = processEvent->vm;
@@ -5057,6 +5393,12 @@ static void qemuProcessEventHandler(void *data, void *opaque)
case QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED:
processRdmaGidStatusChangedEvent(vm, processEvent->data);
break;
+ case QEMU_PROCESS_EVENT_USB_REMOVED:
+ processUSBRemovedEvent(driver, vm, processEvent->data);
+ break;
+ case QEMU_PROCESS_EVENT_USB_ADDED:
+ processUSBAddedEvent(driver, vm, processEvent->data);
+ break;
case QEMU_PROCESS_EVENT_LAST:
break;
}
--
2.23.0
7:08 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: handle usb hostdev add/del on device DELETED_EVENT
Now device DELETED_EVENT can have 2 causes. First is detaching device by
client just as before. Second is attaching/detaching hostdev assigned to
guest on node. In the latter case we reattach hostdev to guest.
When device is detached from node we remove device from qemu and insert
new dummy one with unset bus and device values and have missing flag set
to true. This is on par with case when we start domain with
startupPolicy = optional and device is missing on node.
When device is attached to node back we remove dummy device from qemu
and insert proper one with discovered bus and device and missing flag is
appropriately set to false.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 12 ++-----
src/qemu/qemu_hotplug.c | 73 ++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_hotplug.h | 4 +++
src/util/virhostdev.c | 6 ++--
4 files changed, 72 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 33b75a3c71..352f6d0376 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4677,7 +4677,6 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
const char *devAlias)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- virDomainDeviceDef dev;
VIR_DEBUG("Removing device %s from domain %p %s",
devAlias, vm, vm->def->name);
@@ -4690,15 +4689,10 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
goto endjob;
}
- if (STRPREFIX(devAlias, "vcpu")) {
+ if (STRPREFIX(devAlias, "vcpu"))
qemuDomainRemoveVcpuAlias(driver, vm, devAlias);
- } else {
- if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0)
- goto endjob;
-
- if (qemuDomainRemoveDevice(driver, vm, &dev) < 0)
- goto endjob;
- }
+ else if (qemuDomainRemoveDeviceAlias(driver, vm, devAlias) < 0)
+ goto endjob;
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)
< 0)
VIR_WARN("unable to save domain status after removing device %s",
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 08e60dcd0e..197b2b2fd7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2437,8 +2437,10 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
bool teardownlabel = false;
bool teardowndevice = false;
int ret = -1;
+ bool reattaching = hostdev->deleteCause ==
VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING;
- if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
+ if (!reattaching &&
+ virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
return -1;
if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, &hostdev, 1, 0)
< 0)
@@ -2463,7 +2465,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
if (!(devstr = qemuBuildUSBHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
goto cleanup;
- if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0)
+ if (!reattaching && VIR_REALLOC_N(vm->def->hostdevs,
vm->def->nhostdevs+1) < 0)
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
@@ -2476,7 +2478,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
if (ret < 0)
goto cleanup;
- vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
+ if (!reattaching)
+ vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
ret = 0;
cleanup:
@@ -2491,7 +2494,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
VIR_WARN("Unable to remove host device from /dev");
if (added)
qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1);
- virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
+ if (!reattaching)
+ virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
}
VIR_FREE(devstr);
return ret;
@@ -4366,7 +4370,8 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver,
virDomainHostdevDefPtr hostdev)
{
qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1);
- qemuDomainReleaseDeviceAddress(vm, hostdev->info);
+ if (hostdev->deleteCause != VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING)
+ qemuDomainReleaseDeviceAddress(vm, hostdev->info);
}
static void
@@ -4408,6 +4413,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
char *drivealias = NULL;
char *objAlias = NULL;
bool is_vfio = false;
+ bool reattaching = hostdev->deleteCause ==
VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING;
VIR_DEBUG("Removing host device %s from domain %p %s",
hostdev->info->alias, vm, vm->def->name);
@@ -4454,16 +4460,25 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
}
}
- for (i = 0; i < vm->def->nhostdevs; i++) {
- if (vm->def->hostdevs[i] == hostdev) {
- virDomainHostdevRemove(vm->def, i);
- break;
+ if (!reattaching) {
+ for (i = 0; i < vm->def->nhostdevs; i++) {
+ if (vm->def->hostdevs[i] == hostdev) {
+ virDomainHostdevRemove(vm->def, i);
+ break;
+ }
}
}
virDomainAuditHostdev(vm, hostdev, "detach", true);
- if (!is_vfio &&
+ /*
+ * In case of reattaching (when usb is detached from host) the attempt to
+ * restore label will fail. But we don't need to restore the label! In case
+ * of separate mount namespace for the domain we remove device file later
+ * in this function. In case of global mount namespace the device file is
+ * deleted or being deleted by systemd.
+ */
+ if (!is_vfio && !reattaching &&
qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0)
VIR_WARN("Failed to restore host device labelling");
@@ -4497,7 +4512,13 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
break;
}
- virDomainHostdevDefFree(hostdev);
+ if (reattaching) {
+ virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
+ usbsrc->bus = 0;
+ usbsrc->device = 0;
+ } else {
+ virDomainHostdevDefFree(hostdev);
+ }
if (net) {
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
@@ -6565,3 +6586,33 @@ qemuDomainSetVcpuInternal(virQEMUDriverPtr driver,
virBitmapFree(livevcpus);
return ret;
}
+
+
+int
+qemuDomainRemoveDeviceAlias(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *devAlias)
+{
+ virDomainDeviceDef dev;
+
+ if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0)
+ return -1;
+
+ if (dev.type == VIR_DOMAIN_DEVICE_HOSTDEV &&
+ dev.data.hostdev->deleteCause == VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING)
{
+ virDomainHostdevDefPtr hostdev = dev.data.hostdev;
+
+ if (qemuDomainRemoveHostDevice(driver, vm, hostdev) < 0)
+ return -1;
+
+ if (qemuDomainAttachHostDevice(driver, vm, hostdev) < 0)
+ return -1;
+
+ hostdev->deleteCause = 0;
+ } else {
+ if (qemuDomainRemoveDevice(driver, vm, &dev) < 0)
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index bf812eab1a..f85e5db2fe 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -151,3 +151,7 @@ int qemuDomainSetVcpuInternal(virQEMUDriverPtr driver,
virDomainDefPtr persistentDef,
virBitmapPtr vcpus,
bool state);
+
+int qemuDomainRemoveDeviceAlias(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *devAlias);
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index d710193b94..a2c0c07eb9 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1429,8 +1429,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
}
out:
- if (!*usb)
- hostdev->missing = true;
+ hostdev->missing = !*usb;
return 0;
}
@@ -1472,7 +1471,8 @@ virHostdevPrepareUSBDevices(virHostdevManagerPtr mgr,
if (hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_OPTIONAL ||
(hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_REQUISITE &&
- !coldBoot))
+ !coldBoot) ||
+ hostdev->deleteCause == VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING)
required = false;
if (virHostdevFindUSBDevice(hostdev, required, &usb) < 0)
--
2.23.0
7:08 a.m.
New subject: [libvirt] [PATCH 4/6] conf: parse hostdev missing flag
We want to keep this flag across libvirtd restarts.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/conf/domain_conf.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7a342bb91..9a2385e31c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7533,6 +7533,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb;
VIR_AUTOFREE(char *) startupPolicy = NULL;
VIR_AUTOFREE(char *) autoAddress = NULL;
+ VIR_AUTOFREE(char *) missing = NULL;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -7550,6 +7551,11 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
usbsrc->autoAddress = true;
}
+ if ((missing = virXMLPropString(node, "missing"))) {
+ if (STREQ(missing, "yes"))
+ def->missing = true;
+ }
+
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized*/
got_product = false;
--
2.23.0
7:08 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: handle libvirtd restarts during hostdev reattaching
We need to handle case when libvirtd is restarted after qemu device is
deleted in order to reattach hostdev and before device deleted event is
delivired. First we need to save delete_cause in status xml so that if
event is delivered after libvirtd started back we can proceed with
reattaching. Second if we missed the event we need to continue with
reattaching too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_driver.c | 20 ++++++++++++++++++--
src/qemu/qemu_process.c | 6 ++----
4 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a2385e31c..ecab83147e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1236,6 +1236,13 @@ VIR_ENUM_IMPL(virDomainShmemModel,
"ivshmem-doorbell",
);
+VIR_ENUM_IMPL(virDomainHostdevDeleteCause,
+ VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_LAST,
+ "none",
+ "client",
+ "reattaching"
+);
+
VIR_ENUM_IMPL(virDomainLaunchSecurity,
VIR_DOMAIN_LAUNCH_SECURITY_LAST,
"",
@@ -7534,6 +7541,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
VIR_AUTOFREE(char *) startupPolicy = NULL;
VIR_AUTOFREE(char *) autoAddress = NULL;
VIR_AUTOFREE(char *) missing = NULL;
+ VIR_AUTOFREE(char *) deleteCause = NULL;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -7556,6 +7564,18 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
def->missing = true;
}
+ if ((deleteCause = virXMLPropString(node, "deleteCause"))) {
+ def->deleteCause =
+ virDomainHostdevDeleteCauseTypeFromString(deleteCause);
+
+ if (def->deleteCause <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown deleteCause '%s'"),
+ deleteCause);
+ goto out;
+ }
+ }
+
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized*/
got_product = false;
@@ -24911,6 +24931,12 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
if (def->missing && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
virBufferAddLit(buf, " missing='yes'");
+
+ if (def->deleteCause && (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)) {
+ const char *deleteCause;
+ deleteCause = virDomainHostdevDeleteCauseTypeToString(def->deleteCause);
+ virBufferAsprintf(buf, " deleteCause='%s'", deleteCause);
+ }
}
if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 49392d0286..a2de4c7657 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -333,6 +333,7 @@ typedef enum {
VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_LAST
} virDomainHostdevDeleteCauseType;
+VIR_ENUM_DECL(virDomainHostdevDeleteCause);
/* basic device for direct passthrough */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 352f6d0376..b912766445 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5265,11 +5265,14 @@ processUSBAddedEvent(virQEMUDriverPtr driver,
struct qemuUdevUSBAddData *data)
{
virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainHostdevDefPtr hostdev;
size_t i;
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
+ virObjectUnref(cfg);
return;
+ }
if (!virDomainObjIsActive(vm)) {
VIR_DEBUG("Domain is not running");
@@ -5295,8 +5298,13 @@ processUSBAddedEvent(virQEMUDriverPtr driver,
if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
goto cleanup;
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)
< 0)
+ VIR_WARN("unable to save domain status after reattaching device %s",
+ hostdev->info->alias);
+
cleanup:
qemuDomainObjEndJob(driver, vm);
+ virObjectUnref(cfg);
}
@@ -5307,10 +5315,13 @@ processUSBRemovedEvent(virQEMUDriverPtr driver,
{
size_t i;
virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainHostdevDefPtr hostdev;
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
+ virObjectUnref(cfg);
return;
+ }
if (!virDomainObjIsActive(vm)) {
VIR_DEBUG("Domain is not running");
@@ -5335,8 +5346,13 @@ processUSBRemovedEvent(virQEMUDriverPtr driver,
if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
goto cleanup;
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)
< 0)
+ VIR_WARN("unable to save domain status after reattaching device %s",
+ hostdev->info->alias);
+
cleanup:
qemuDomainObjEndJob(driver, vm);
+ virObjectUnref(cfg);
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c9921646e9..5cb97de4f3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3720,7 +3720,6 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- virDomainDeviceDef dev;
const char **qemuDevices;
char **old;
char **tmp;
@@ -3735,10 +3734,9 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver,
if ((tmp = old)) {
while (*tmp) {
if (!virStringListHasString(qemuDevices, *tmp) &&
- virDomainDefFindDevice(vm->def, *tmp, &dev, false) == 0
&&
- qemuDomainRemoveDevice(driver, vm, &dev) < 0) {
+ qemuDomainRemoveDeviceAlias(driver, vm, *tmp) < 0)
goto cleanup;
- }
+
tmp++;
}
}
--
2.23.0
7:08 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: handle hostdev add/del when libvirtd is down
Somebody can easily detach hostdev on node while libvirtd is being
stopped. Also hostdev can be attached or detached and attached back and
so forth. Let's handle such cases.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_process.c | 49 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5cb97de4f3..f70f0ae251 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3747,6 +3747,52 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver,
return ret;
}
+
+static int
+qemuProcessReattachUSBDevices(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+{
+ size_t i;
+
+ for (i = 0; i < vm->def->nhostdevs; i++) {
+ virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+ virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
+ virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
+ bool found;
+
+
+ if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+ continue;
+
+ if (hostdev->missing) {
+ int num;
+
+ if ((num = virUSBDeviceFindByVendor(usbsrc->vendor, usbsrc->product,
+ NULL, false, NULL)) < 0)
+ return -1;
+
+ found = num != 0;
+ } else {
+ virUSBDevicePtr usb;
+
+ if (virUSBDeviceFindByBus(usbsrc->bus, usbsrc->device,
+ NULL, false, &usb) < 0)
+ return -1;
+
+ found = usb != NULL;
+ virUSBDeviceFree(usb);
+ }
+
+ dev.data.hostdev = hostdev;
+ if ((hostdev->missing ^ !found) &&
+ qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int
qemuDomainPerfRestart(virDomainObjPtr vm)
{
@@ -8204,6 +8250,9 @@ qemuProcessReconnect(void *opaque)
if (qemuProcessUpdateDevices(driver, obj) < 0)
goto error;
+ if (qemuProcessReattachUSBDevices(driver, obj) < 0)
+ goto error;
+
if (qemuRefreshPRManagerState(driver, obj) < 0)
goto error;
--
2.23.0
2:40 p.m.
Hi Nikolay,
I was going to review these patches but first I followed up in the
discussions at [1]. Daniel made a few points there:
---
The problems around host USB device passthrough are conceptually similar
to the problems of hots PCI device passthrough.
In both cases we cannot assume the device present on the source device
exists on the target device in the same way.
(...)
For PCI devices we simply refuse to initiate the migration if any host
PCI devices are attached. The mgmt app has to hot-unplug all devices
before migration, and hot-plug new devices after migration if desired.
I'm inclined to suggest that same approach of hotunplug + hotplug either
side of migration is the only viable option for host USB devices too.
(...)
---
My question: is this patch series still alive? Seems to me that you made
these changes based at least partially with migration in mind, and
the migration solution Libvirt adopted is to rely on upper management
to detach/reattach hostdev devices prior/before migration.
Thanks,
DHB
On 8/30/19 9:08 AM, Nikolay Shirokovskiy wrote:
Hi, all.
This patch series aims to handle case when hostdev usb is detached and then
attached back on node. Currently libvirtd does not track these events so that in
the result hostdev usb is not usable.
On the first glace it make sense to delete device from qemu when hostdev is
detached on node and then add device back when hostdev is attached back. But
these series adds a different behaviour. On detaching device is deleted from
qemu and dummy (missing hostaddr and hostbus values) hostdev is inserted, then
on attaching device on node dummy device is deleted and proper one is inserted.
This way we have the same semantics of 'missing' flag for hostdev as in case of
hostdev missing on startup/migration to different host. Also with such
approach we additionally handle cases when hostdev is missing on startup but
later is attached to node.
Known issues:
The last patch tries to handle case when hostdev attached/detached when
libvirtd is down but if fails in one scenario. If hostdev is detached and thus
dummy device in inserted in qemu then if libvirtd is down and hostdev is
attached to node then qemu tries to bind appeared hostdev to the dummy device
but fails as device is missing in qemu's mount namespace. Later when libvirtd
is up and tries to replace dummy device with actual one the device_add fails
because libusb the qemu uses tracks that deviced was missed ealier. For this
reason I proposed to get rid of dummy device in RFC [1]. Alternatively libusb
can be fixed.
[1] https://www.redhat.com/archives/libvir-list/2019-August/msg01396.html
Nikolay Shirokovskiy (6):
qemu: track hostdev delete cause
qemu: handle usb hostdev add/del udev events
qemu: handle usb hostdev add/del on device DELETED_EVENT
conf: parse hostdev missing flag
qemu: handle libvirtd restarts during hostdev reattaching
qemu: handle hostdev add/del when libvirtd is down
src/conf/domain_conf.c | 32 ++++
src/conf/domain_conf.h | 10 ++
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_domain.c | 2 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 376 +++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_hotplug.c | 94 ++++++++--
src/qemu/qemu_hotplug.h | 7 +-
src/qemu/qemu_process.c | 55 +++++-
src/util/virhostdev.c | 6 +-
tests/qemuhotplugtest.c | 2 +-
12 files changed, 558 insertions(+), 33 deletions(-)
1:51 a.m.
On 03.09.2019 22:40, Daniel Henrique Barboza wrote:
Hi Nikolay,
I was going to review these patches but first I followed up in the
discussions at [1]. Daniel made a few points there:
---
The problems around host USB device passthrough are conceptually similar
to the problems of hots PCI device passthrough.
In both cases we cannot assume the device present on the source device
exists on the target device in the same way.
(...)
For PCI devices we simply refuse to initiate the migration if any host
PCI devices are attached. The mgmt app has to hot-unplug all devices
before migration, and hot-plug new devices after migration if desired.
I'm inclined to suggest that same approach of hotunplug + hotplug either
side of migration is the only viable option for host USB devices too.
(...)
---
My question: is this patch series still alive? Seems to me that you made
these changes based at least partially with migration in mind, and
the migration solution Libvirt adopted is to rely on upper management
to detach/reattach hostdev devices prior/before migration.
Hi, Daniel.
I'm going to send 2nd version of the patchset without using dummy device
as suggested by Dan.
Nikolay
Thanks,
DHB
On 8/30/19 9:08 AM, Nikolay Shirokovskiy wrote:
> Hi, all.
>
> This patch series aims to handle case when hostdev usb is detached and then
> attached back on node. Currently libvirtd does not track these events so that in
> the result hostdev usb is not usable.
>
> On the first glace it make sense to delete device from qemu when hostdev is
> detached on node and then add device back when hostdev is attached back. But
> these series adds a different behaviour. On detaching device is deleted from
> qemu and dummy (missing hostaddr and hostbus values) hostdev is inserted, then
> on attaching device on node dummy device is deleted and proper one is inserted.
> This way we have the same semantics of 'missing' flag for hostdev as in case
of
> hostdev missing on startup/migration to different host. Also with such
> approach we additionally handle cases when hostdev is missing on startup but
> later is attached to node.
>
> Known issues:
>
> The last patch tries to handle case when hostdev attached/detached when
> libvirtd is down but if fails in one scenario. If hostdev is detached and thus
> dummy device in inserted in qemu then if libvirtd is down and hostdev is
> attached to node then qemu tries to bind appeared hostdev to the dummy device
> but fails as device is missing in qemu's mount namespace. Later when libvirtd
> is up and tries to replace dummy device with actual one the device_add fails
> because libusb the qemu uses tracks that deviced was missed ealier. For this
> reason I proposed to get rid of dummy device in RFC [1]. Alternatively libusb
> can be fixed.
>
> [1] https://www.redhat.com/archives/libvir-list/2019-August/msg01396.html
>
> Nikolay Shirokovskiy (6):
> qemu: track hostdev delete cause
> qemu: handle usb hostdev add/del udev events
> qemu: handle usb hostdev add/del on device DELETED_EVENT
> conf: parse hostdev missing flag
> qemu: handle libvirtd restarts during hostdev reattaching
> qemu: handle hostdev add/del when libvirtd is down
>
> src/conf/domain_conf.c | 32 ++++
> src/conf/domain_conf.h | 10 ++
> src/qemu/Makefile.inc.am | 2 +
> src/qemu/qemu_conf.h | 3 +
> src/qemu/qemu_domain.c | 2 +
> src/qemu/qemu_domain.h | 2 +
> src/qemu/qemu_driver.c | 376 +++++++++++++++++++++++++++++++++++++--
> src/qemu/qemu_hotplug.c | 94 ++++++++--
> src/qemu/qemu_hotplug.h | 7 +-
> src/qemu/qemu_process.c | 55 +++++-
> src/util/virhostdev.c | 6 +-
> tests/qemuhotplugtest.c | 2 +-
> 12 files changed, 558 insertions(+), 33 deletions(-)
>
1905
days inactive
1910
days old
8 comments
2 participants
participants (2)
-
Daniel Henrique Barboza -
Nikolay Shirokovskiy