src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++++++++++++ src/libxl/libxl_conf.h | 4 ++++ src/libxl/libxl_domain.c | 25 +++++++++++++++++++++++++ src/libxl/libxl_driver.c | 2 +- 4 files changed, 63 insertions(+), 1 deletion(-)
When a user does not explicitly set a <driver> in the disk config,
libvirt defers selection of a default to libxl. This approach works
fine when starting a domain with such configuration or attaching a
disk to a running domain. But when detaching such a disk, libxl
will fail with "unrecognized disk backend type: 0". libxl makes no
attempt to recalculate a default backend (driver) on detach and
simply fails when uninitialized.
This patch updates the libvirt disk config with the backend selected
by libxl when starting a domain or attaching a disk to a running
domain. Another benefit of this approach is that the live XML is
also updated with the backend driver selected by libxl.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++++++++++++
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_domain.c | 25 +++++++++++++++++++++++++
src/libxl/libxl_driver.c | 2 +-
4 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b5186f2..6ef7570 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -851,6 +851,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
* xl-disk-configuration.txt in the xen documentation and let
* libxl pick a suitable backend.
*/
+ virDomainDiskSetFormat(l_disk, VIR_STORAGE_FILE_RAW);
x_disk->format = LIBXL_DISK_FORMAT_RAW;
x_disk->backend = LIBXL_DISK_BACKEND_UNKNOWN;
}
@@ -913,6 +914,38 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
return -1;
}
+/*
+ * Update libvirt disk config with libxl disk config.
+ *
+ * This function can be used to update the libvirt disk config with default
+ * values selected by libxl. Currently only the backend type is selected by
+ * libxl when not explicitly specified by the user.
+ */
+void
+libxlUpdateDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
+{
+ const char *driver = NULL;
+
+ if (virDomainDiskGetDriver(l_disk))
+ return;
+
+ switch (x_disk->backend) {
+ case LIBXL_DISK_BACKEND_QDISK:
+ driver = "qemu";
+ break;
+ case LIBXL_DISK_BACKEND_TAP:
+ driver = "tap";
+ break;
+ case LIBXL_DISK_BACKEND_PHY:
+ driver = "phy";
+ break;
+ case LIBXL_DISK_BACKEND_UNKNOWN:
+ break;
+ }
+ if (driver)
+ ignore_value(virDomainDiskSetDriver(l_disk, driver));
+}
+
int
libxlMakeNic(virDomainDefPtr def,
virDomainNetDefPtr l_nic,
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 69d7885..732a1d2 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -175,6 +175,10 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
int
libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
+
+void
+libxlUpdateDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
+
int
libxlMakeNic(virDomainDefPtr def,
virDomainNetDefPtr l_nic,
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ed73cd2..0168777 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1067,6 +1067,30 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def, libxl_domain_config *d_config)
}
}
+static void
+libxlDomainUpdateDiskParams(virDomainDefPtr def, libxl_ctx *ctx)
+{
+ libxl_device_disk *disks;
+ int num_disks = 0;
+ size_t i;
+ int idx;
+
+ disks = libxl_device_disk_list(ctx, def->id, &num_disks);
+ if (!disks)
+ return;
+
+ for (i = 0; i < num_disks; i++) {
+ if ((idx = virDomainDiskIndexByName(def, disks[i].vdev, false)) < 0)
+ continue;
+
+ libxlUpdateDisk(def->disks[idx], &disks[i]);
+ }
+
+ for (i = 0; i < num_disks; i++)
+ libxl_device_disk_dispose(&disks[i]);
+ VIR_FREE(disks);
+}
+
#ifdef LIBXL_HAVE_DEVICE_CHANNEL
static void
libxlDomainCreateChannelPTY(virDomainDefPtr def, libxl_ctx *ctx)
@@ -1310,6 +1334,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
goto destroy_dom;
libxlDomainCreateIfaceNames(vm->def, &d_config);
+ libxlDomainUpdateDiskParams(vm->def, cfg->ctx);
#ifdef LIBXL_HAVE_DEVICE_CHANNEL
if (vm->def->nchannels > 0)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3a69720..6dddf9f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3027,7 +3027,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
}
goto cleanup;
}
-
+ libxlUpdateDisk(l_disk, &x_disk);
virDomainDiskInsertPreAlloced(vm->def, l_disk);
} else {
--
2.9.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/06/2017 11:08 PM, Jim Fehlig wrote: > When a user does not explicitly set a <driver> in the disk config, > libvirt defers selection of a default to libxl. This approach works > fine when starting a domain with such configuration or attaching a > disk to a running domain. But when detaching such a disk, libxl > will fail with "unrecognized disk backend type: 0". libxl makes no > attempt to recalculate a default backend (driver) on detach and > simply fails when uninitialized. > > This patch updates the libvirt disk config with the backend selected > by libxl when starting a domain or attaching a disk to a running > domain. Another benefit of this approach is that the live XML is > also updated with the backend driver selected by libxl. > > Signed-off-by: Jim Fehlig <jfehlig@suse.com> > --- > src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++++++++++++ > src/libxl/libxl_conf.h | 4 ++++ > src/libxl/libxl_domain.c | 25 +++++++++++++++++++++++++ > src/libxl/libxl_driver.c | 2 +- > 4 files changed, 63 insertions(+), 1 deletion(-) > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index b5186f2..6ef7570 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -851,6 +851,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) > * xl-disk-configuration.txt in the xen documentation and let > * libxl pick a suitable backend. > */ > + virDomainDiskSetFormat(l_disk, VIR_STORAGE_FILE_RAW); > x_disk->format = LIBXL_DISK_FORMAT_RAW; > x_disk->backend = LIBXL_DISK_BACKEND_UNKNOWN; This doesn't feel right. I know what do you want it here for, but this function is meant to take 'const' disk definition and translate it to libxl definition. Although, I don't have a better suggestion where to put it. > } > @@ -913,6 +914,38 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) > return -1; > } > The rest looks okay. ACK. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/07/2017 03:26 AM, Michal Privoznik wrote: > On 02/06/2017 11:08 PM, Jim Fehlig wrote: >> When a user does not explicitly set a <driver> in the disk config, >> libvirt defers selection of a default to libxl. This approach works >> fine when starting a domain with such configuration or attaching a >> disk to a running domain. But when detaching such a disk, libxl >> will fail with "unrecognized disk backend type: 0". libxl makes no >> attempt to recalculate a default backend (driver) on detach and >> simply fails when uninitialized. >> >> This patch updates the libvirt disk config with the backend selected >> by libxl when starting a domain or attaching a disk to a running >> domain. Another benefit of this approach is that the live XML is >> also updated with the backend driver selected by libxl. >> >> Signed-off-by: Jim Fehlig <jfehlig@suse.com> >> --- >> src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++++++++++++ >> src/libxl/libxl_conf.h | 4 ++++ >> src/libxl/libxl_domain.c | 25 +++++++++++++++++++++++++ >> src/libxl/libxl_driver.c | 2 +- >> 4 files changed, 63 insertions(+), 1 deletion(-) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index b5186f2..6ef7570 100644 >> --- a/src/libxl/libxl_conf.c >> +++ b/src/libxl/libxl_conf.c >> @@ -851,6 +851,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) >> * xl-disk-configuration.txt in the xen documentation and let >> * libxl pick a suitable backend. >> */ >> + virDomainDiskSetFormat(l_disk, VIR_STORAGE_FILE_RAW); >> x_disk->format = LIBXL_DISK_FORMAT_RAW; >> x_disk->backend = LIBXL_DISK_BACKEND_UNKNOWN; > > This doesn't feel right. I know what do you want it here for, but this > function is meant to take 'const' disk definition and translate it to > libxl definition. Although, I don't have a better suggestion where to > put it. How about in the UpdateDisk function introduced in this patch? E.g. update it to raw if current format is VIR_STORAGE_FILE_NONE? Regards, Jim > >> } >> @@ -913,6 +914,38 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) >> return -1; >> } >> > > The rest looks okay. ACK. > > Michal > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/07/2017 08:30 AM, Jim Fehlig wrote: > On 02/07/2017 03:26 AM, Michal Privoznik wrote: >> On 02/06/2017 11:08 PM, Jim Fehlig wrote: >>> When a user does not explicitly set a <driver> in the disk config, >>> libvirt defers selection of a default to libxl. This approach works >>> fine when starting a domain with such configuration or attaching a >>> disk to a running domain. But when detaching such a disk, libxl >>> will fail with "unrecognized disk backend type: 0". libxl makes no >>> attempt to recalculate a default backend (driver) on detach and >>> simply fails when uninitialized. >>> >>> This patch updates the libvirt disk config with the backend selected >>> by libxl when starting a domain or attaching a disk to a running >>> domain. Another benefit of this approach is that the live XML is >>> also updated with the backend driver selected by libxl. >>> >>> Signed-off-by: Jim Fehlig <jfehlig@suse.com> >>> --- >>> src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++++++++++++ >>> src/libxl/libxl_conf.h | 4 ++++ >>> src/libxl/libxl_domain.c | 25 +++++++++++++++++++++++++ >>> src/libxl/libxl_driver.c | 2 +- >>> 4 files changed, 63 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >>> index b5186f2..6ef7570 100644 >>> --- a/src/libxl/libxl_conf.c >>> +++ b/src/libxl/libxl_conf.c >>> @@ -851,6 +851,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, >>> libxl_device_disk *x_disk) >>> * xl-disk-configuration.txt in the xen documentation and let >>> * libxl pick a suitable backend. >>> */ >>> + virDomainDiskSetFormat(l_disk, VIR_STORAGE_FILE_RAW); >>> x_disk->format = LIBXL_DISK_FORMAT_RAW; >>> x_disk->backend = LIBXL_DISK_BACKEND_UNKNOWN; >> >> This doesn't feel right. I know what do you want it here for, but this >> function is meant to take 'const' disk definition and translate it to >> libxl definition. Although, I don't have a better suggestion where to >> put it. > > How about in the UpdateDisk function introduced in this patch? E.g. update it to > raw if current format is VIR_STORAGE_FILE_NONE? After thinking about it some more, seems the best place to set the default when format is VIR_STORAGE_FILE_NONE is in the device post-parse callback. I've sent a V2 along those lines https://www.redhat.com/archives/libvir-list/2017-February/msg00248.html Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.