1:46 p.m.
V2 of
https://www.redhat.com/archives/libvir-list/2017-February/msg00185.html
Patch1 is new in V2 and essentially moves default setting of disk
format from domain build time to device post-parse, which addresses
Michal's comment from V1.
Patch2 is mostly unchanged. The hunk setting format of virDomainDiskDef
in libxlMakeDisk is removed since that is now handled by patch1.
Jim Fehlig (2):
libxl: set default disk format in device post-parse
libxl: fix disk detach when <driver> not specified
src/libxl/libxl_conf.c | 42 ++++++++++++++++++++++++++++++++++--------
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_domain.c | 32 +++++++++++++++++++++++++++++++-
src/libxl/libxl_driver.c | 1 +
4 files changed, 70 insertions(+), 9 deletions(-)
--
2.9.2
1:46 p.m.
New subject: [libvirt] [PATCH V2 1/2] libxl: set default disk format in device post-parse
When starting a domian, a libxl_domain_config object is created from
virDomainDef. Any virDomainDiskDef devices with a format of
VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
corresponding libxl_disk_device, but the virDomainDiskDef format is
never updated to reflect the change.
A better place to set a default format for disk devices is the
device post-parse callback, ensuring the virDomainDiskDef object
reflects the default format.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 10 ++--------
src/libxl/libxl_domain.c | 7 ++++++-
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b5186f2..da63105 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -765,8 +765,6 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
x_disk->format = LIBXL_DISK_FORMAT_VHD;
x_disk->backend = LIBXL_DISK_BACKEND_TAP;
break;
- case VIR_STORAGE_FILE_NONE:
- /* No subtype specified, default to raw/tap */
case VIR_STORAGE_FILE_RAW:
x_disk->format = LIBXL_DISK_FORMAT_RAW;
x_disk->backend = LIBXL_DISK_BACKEND_TAP;
@@ -802,8 +800,6 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
case VIR_STORAGE_FILE_VHD:
x_disk->format = LIBXL_DISK_FORMAT_VHD;
break;
- case VIR_STORAGE_FILE_NONE:
- /* No subtype specified, default to raw */
case VIR_STORAGE_FILE_RAW:
x_disk->format = LIBXL_DISK_FORMAT_RAW;
break;
@@ -816,8 +812,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
return -1;
}
} else if (STREQ(driver, "file")) {
- if (format != VIR_STORAGE_FILE_NONE &&
- format != VIR_STORAGE_FILE_RAW) {
+ if (format != VIR_STORAGE_FILE_RAW) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight does not support disk format %s
"
"with disk driver %s"),
@@ -828,8 +823,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
x_disk->format = LIBXL_DISK_FORMAT_RAW;
x_disk->backend = LIBXL_DISK_BACKEND_QDISK;
} else if (STREQ(driver, "phy")) {
- if (format != VIR_STORAGE_FILE_NONE &&
- format != VIR_STORAGE_FILE_RAW) {
+ if (format != VIR_STORAGE_FILE_RAW) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight does not support disk format %s
"
"with disk driver %s"),
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ed73cd2..c0ace33 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -362,16 +362,21 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
}
}
- /* for network-based disks, set 'qemu' as the default driver */
if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
virDomainDiskDefPtr disk = dev->data.disk;
int actual_type = virStorageSourceGetActualType(disk->src);
+ int format = virDomainDiskGetFormat(disk);
+ /* for network-based disks, set 'qemu' as the default driver */
if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
if (!virDomainDiskGetDriver(disk) &&
virDomainDiskSetDriver(disk, "qemu") < 0)
return -1;
}
+
+ /* xl.cfg default format is raw. See xl-disk-configuration(5) */
+ if (format == VIR_STORAGE_FILE_NONE)
+ virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
}
return 0;
--
2.9.2
1:18 a.m.
New subject: [libvirt] [PATCH V2 1/2] libxl: set default disk format in device post-parse
On 02/07/2017 08:46 PM, Jim Fehlig wrote:
When starting a domian, a libxl_domain_config object is created from
virDomainDef. Any virDomainDiskDef devices with a format of
VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
corresponding libxl_disk_device, but the virDomainDiskDef format is
never updated to reflect the change.
A better place to set a default format for disk devices is the
device post-parse callback, ensuring the virDomainDiskDef object
reflects the default format.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 10 ++--------
src/libxl/libxl_domain.c | 7 ++++++-
2 files changed, 8 insertions(+), 9 deletions(-)
ACK.
BTW looks like you forgot about one other location:
src/libxl/libxl_driver.c=5452=libxlDomainBlockStatsGatherSingle(virDomainObjPtr vm,
--
src/libxl/libxl_driver.c-5469-
src/libxl/libxl_driver.c-5470- if (STREQ(disk_drv, "phy")) {
src/libxl/libxl_driver.c-5471- if (disk_fmt != VIR_STORAGE_FILE_RAW &&
src/libxl/libxl_driver.c:5472: disk_fmt != VIR_STORAGE_FILE_NONE) {
src/libxl/libxl_driver.c-5473- virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
src/libxl/libxl_driver.c-5474- _("unsupported format
%s"),
src/libxl/libxl_driver.c-5475-
virStorageFileFormatTypeToString(disk_fmt));
Michal
10:26 a.m.
New subject: [libvirt] [PATCH V2 1/2] libxl: set default disk format in device post-parse
Michal Privoznik wrote:
On 02/07/2017 08:46 PM, Jim Fehlig wrote:
> When starting a domian, a libxl_domain_config object is created from
> virDomainDef. Any virDomainDiskDef devices with a format of
> VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
> corresponding libxl_disk_device, but the virDomainDiskDef format is
> never updated to reflect the change.
>
> A better place to set a default format for disk devices is the
> device post-parse callback, ensuring the virDomainDiskDef object
> reflects the default format.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_conf.c | 10 ++--------
> src/libxl/libxl_domain.c | 7 ++++++-
> 2 files changed, 8 insertions(+), 9 deletions(-)
ACK.
BTW looks like you forgot about one other location:
src/libxl/libxl_driver.c=5452=libxlDomainBlockStatsGatherSingle(virDomainObjPtr vm,
--
src/libxl/libxl_driver.c-5469-
src/libxl/libxl_driver.c-5470- if (STREQ(disk_drv, "phy")) {
src/libxl/libxl_driver.c-5471- if (disk_fmt != VIR_STORAGE_FILE_RAW &&
src/libxl/libxl_driver.c:5472: disk_fmt != VIR_STORAGE_FILE_NONE) {
src/libxl/libxl_driver.c-5473- virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
src/libxl/libxl_driver.c-5474- _("unsupported format
%s"),
src/libxl/libxl_driver.c-5475-
virStorageFileFormatTypeToString(disk_fmt));
Ah, thanks. I've added that one to the patch and pushed the series.
Regards,
Jim
1:46 p.m.
New subject: [libvirt] [PATCH V2 2/2] libxl: fix disk detach when <driver> not specified
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(a)suse.com>
---
src/libxl/libxl_conf.c | 32 ++++++++++++++++++++++++++++++++
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_domain.c | 25 +++++++++++++++++++++++++
src/libxl/libxl_driver.c | 1 +
4 files changed, 62 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index da63105..4e38676 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -907,6 +907,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
+libxlUpdateDiskDef(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..b944f6c 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
+libxlUpdateDiskDef(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 c0ace33..57ec661 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1072,6 +1072,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;
+
+ libxlUpdateDiskDef(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)
@@ -1315,6 +1339,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..e7827cc 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3028,6 +3028,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
goto cleanup;
}
+ libxlUpdateDiskDef(l_disk, &x_disk);
virDomainDiskInsertPreAlloced(vm->def, l_disk);
} else {
--
2.9.2
1:18 a.m.
New subject: [libvirt] [PATCH V2 2/2] libxl: fix disk detach when <driver> not specified
On 02/07/2017 08:46 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(a)suse.com>
---
src/libxl/libxl_conf.c | 32 ++++++++++++++++++++++++++++++++
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_domain.c | 25 +++++++++++++++++++++++++
src/libxl/libxl_driver.c | 1 +
4 files changed, 62 insertions(+)
ACK
Michal
2846
days inactive
2848
days old
5 comments
2 participants
participants (2)
-
Jim Fehlig -
Michal Privoznik