Re: [PATCH v2 0/4] Add support for two i386 pm options which control acpi hotplug
On Thu, 19 Aug 2021, Ani Sinha wrote: > Hi: > > I added some negative xml2argv tests as well as new xml2xml tests. In the > process, > I also fixed a bug where I had not added appropriate code to generate the > output > xml correctly. > The patch series is sent again as v2. Kindly, please provide inputs and > review them. > > [PATCH v2 1/4] pm/i386: add support for two options that controls > [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi > [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi > [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug Ping ... please provide some review on this patchset.
Re: [PATCH 3/8] ch_monitor: Update virCHMonitorGet to handle accept a response
On Fri, 2021-08-27 at 17:30 +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 26, 2021 at 02:59:17PM -0700, William Douglas wrote:
> > The virCHMonitorGet function needed to be able to return data from
> > the
> > hypervisor. This functionality is needed in order for the driver to
> > support PTY enablement and getting details about the VM state.
> >
> > Signed-off-by: William Douglas
> > ---
> > src/ch/ch_monitor.c | 44
> > ++--
> > 1 file changed, 42 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> > index b4bc10bfcf..ee7e4683e3 100644
> > --- a/src/ch/ch_monitor.c
> > +++ b/src/ch/ch_monitor.c
> > @@ -611,12 +611,36 @@ virCHMonitorPutNoContent(virCHMonitor *mon,
> > const char *endpoint)
> > return ret;
> > }
> >
> > +struct curl_data {
> > + char *content;
> > + size_t size;
> > +};
> > +
> > +static size_t
> > +curl_callback(void *contents, size_t size, size_t nmemb, void
> > *userp)
> > +{
> > + size_t content_size = size * nmemb;
> > + struct curl_data *data = (struct curl_data *)userp;
> > +
> > + if (content_size == 0)
> > + return content_size;
> > +
> > + data->content = g_realloc(data->content, data->size +
> > content_size);
> > +
> > + memcpy(&(data->content[data->size]), contents, content_size);
> > + data->size += content_size;
> > +
> > + return content_size;
> > +}
> > +
> > static int
> > -virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
> > +virCHMonitorGet(virCHMonitor *mon, const char *endpoint,
> > virJSONValue **response)
> > {
> > g_autofree char *url = NULL;
> > int responseCode = 0;
> > int ret = -1;
> > + struct curl_slist *headers = NULL;
> > + struct curl_data data = {0};
> >
> > url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
> >
> > @@ -628,12 +652,28 @@ virCHMonitorGet(virCHMonitor *mon, const char
> > *endpoint)
> > curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon-
> > >socketpath);
> > curl_easy_setopt(mon->handle, CURLOPT_URL, url);
> >
> > + if (response) {
> > + headers = curl_slist_append(headers, "Accept:
> > application/json");
> > + headers = curl_slist_append(headers, "Content-Type:
> > application/json");
> > + curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER,
> > headers);
> > + curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION,
> > curl_callback);
> > + curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void
> > *));
>
> This bit feels dodgy to me. mon->handle has its lifetime tied to
> the virCHMonitor object, but '' is allocated on the stack.
> IOW, this pointer is persistent, but it goes out of scope.
>
> I guess you're relying on the fact that calls to this method are
> serialized, and we re-write the bad pointer on every call, but
> if 'response' is NULL on the next call we're not going to do that
> re-write.
>
> Can all these options be set unconditionally when the mon->handle
> is first created ?
They could be but the use of curl_easy_reset (which is just outside the
context of this patch I'm noticing, sorry) will cause anything we
initialize the handle with on creation to be reset anyway.
All the calls using the handle lock the mon and then do a
curl_easy_reset on the handle prior to use.
I still understand the dislike for having the handle with potentially
bad pointers in it though. Another option would just be having the
handle exist on the stack though we then lose the live connections but
that wouldn't be an issue for this use case.
Thoughts?
>
> > + }
> > +
> > responseCode = virCHMonitorCurlPerform(mon->handle);
> >
> > virObjectUnlock(mon);
> >
> > - if (responseCode == 200 || responseCode == 204)
> > + if (responseCode == 200 || responseCode == 204) {
> > ret = 0;
> > + if (response) {
> > + data.content = g_realloc(data.content, data.size + 1);
> > + data.content[data.size] = 0;
> > + *response = virJSONValueFromString(data.content);
> > + }
> > + }
> > +
> > + g_free(data.content);
> >
> > return ret;
> > }
>
> Regards,
> Daniel
Re: [PATCH v3 0/2] hw/arm/raspi: Remove deprecated raspi2/raspi3 aliases
On Fri, 27 Aug 2021 at 07:08, Philippe Mathieu-Daudé wrote: > > Since v2: > - updated "Since 6.1" -> "Since 6.2" > > Peter reported CI failure [*] but I can't reproduce: > https://gitlab.com/philmd/qemu/-/pipelines/360227279 'make check' seems to work for me too this time... Applied to target-arm.next, thanks. -- PMM
RFC: revert to external snapshot API
Hi, all. I want to implement reverting to external snapshot functionality. I've checked the mailing list history and found some previous attempts (not sure if this is a complete list of them). [1] was done in 2012 by the Redhat team itself. [2] and [3] was done in 2018. Looks like it was not clear how the API should look like. For example we have disk.qcow2 <- disk.snap1 chain <- disk.snap2 chain (after having snap1 and snap2 snapshots). Now we want to revert to snap1 snapshot. The snapshot state is held by disk.qcow2 image. We can run reverted domain on disk.qcow2 itself but then we lose snap1 (held by disk.qcow2) and snap2 (held by disk.snap1). So we definitely should run on some overlay over disk.qcow2. But then what name should be given to overlay? We should have an option for mgmt to specify this name like in case of snapshots itself. The [1] has some discussion on adding extra options to reverting functionality. Like for example if we revert back to snap2 then having the ability to run from state in disk.snap2 rather than disk.snap1. My opinion is there is no need to as if one wants to revert to the state of disk2.snap2 it can take a snapshot (some snap3). At the same time one needs to be aware that revert operation loses current state and later one can revert only to the states of snapshots. This is the way internal snapshots work and the way one expects external snapshots to work too. The [2] takes an approach of reusing current top image as overlay on revert so that in the above example disk.snap2 will be overlay over disk.qcow2 on reverting to snap1 snapshot. IMHO this is a confusing naming scheme. The [3] suggests a different scheme for naming images. For example after taking snapshot snap1 the chain should be disk.snap1 <- disk.qcow2 which looks very appealing to me. With this naming using the [2] approach is quite natural. Implementing this does not look hard even for a running domain but this is a big change to API and all mgmt need to be aware of (well it could be done safely using a new flag). Anyway we can go on with current image names. In order to specify overlay image name let's introduce new API: int virDomainRevertToSnapshotXML(virDomainSnapshotPtr snapshot, char *xmlDesc, unsigned int flags) with XML like: Having an XML looks like a bit overkill right now but I could not find a better solution. If overlay name is omitted the generated name will be like disk.snap1-1 in the above example to be in alignment with creating a snapshot case. So that disk.snap1* images hold states derived from snap1 state. We can also support reverting to external snapshot thru existing virDomainRevertToSnapshot for those who rely on generated names using this naming scheme. [1] Re: [PATCHv4 4/4] snapshot: qemu: Implement reverting of external snapshots https://listman.redhat.com/archives/libvir-list/2012-November/msg00932.html [2] Re: [PATCH 1/5] snapshot: Implement reverting for external disk snapshots https://listman.redhat.com/archives/libvir-list/2018-November/msg00338.html [3] External disk snapshot paths and the revert operation https://listman.redhat.com/archives/libvir-list/2018-October/msg01110.html
Re: [libvirt PATCH] meson: avoid bogus warnings from clang and g_autoptr
On Tue, Aug 31, 2021 at 05:23:08PM +0200, Pavel Hrdina wrote:
> On Tue, Aug 31, 2021 at 03:08:19PM +0100, Daniel P. Berrangé wrote:
> > Clang has previously had trouble with G_DEFINE_AUTOPTR_CLEANUP_FUNC
> > generated code, thinking it was unused. We turn off -Wunused-function
> > to avoid tripping up on that with CLang.
> >
> > New CLang has started having trouble with g_autoptr now too. In usage
> > scenarios where the variable is set, but never again read, it thinks
> > it is unused not realizing the destructor has useful side effects.
> > For this we have to skip -Wunused-but-set-variable on CLang.
>
> s/CLang/Clang/
>
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > meson.build | 20
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/meson.build b/meson.build
> > index 5af09d319a..dbd70b6483 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -470,6 +470,26 @@ if get_option('warning_level') == '2'
> > supported_cc_flags += [ '-Wno-unused-function' ]
> >endif
> >
> > + # Clang complains about unused variables in many scenarios arelated
>
> s/arelated/related/
>
> > + # to attribute((cleanup)) aka g_auto*
> > + w_unused_but_set_var_args = [ '-Wunused-but-set-variable', '-Werror' ]
> > + w_unused_but_set_var_code = '''
> > +static inline void free_pointer (void *p) {
> > + void **pp = (void**)p;
> > + free (*pp);
> > +}
> > +
> > +int main(void) {
> > + __attribute__((cleanup(free_pointer))) char *buffer = 0x0;
> > + buffer = 0x1;
> > +}
> > + '''
> > + # We previously turned on unused-but-set-variable, so we must turn
> > + # it off again explicitly now.
> > + if not cc.compiles(w_unused_but_set_var_code, args:
> > w_unused_but_set_var_args)
> > +supported_cc_flags += [ '-Wno-unused-but-set-variable' ]
> > + endif
> > +
> > endif
> > add_project_arguments(supported_cc_flags, language: 'c')
>
> Reviewed-by: Pavel Hrdina
Turns out this broke on macOS / FreeBSD with older CLang so will need
a v2.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH] meson: avoid bogus warnings from clang and g_autoptr
On Tue, Aug 31, 2021 at 03:08:19PM +0100, Daniel P. Berrangé wrote:
> Clang has previously had trouble with G_DEFINE_AUTOPTR_CLEANUP_FUNC
> generated code, thinking it was unused. We turn off -Wunused-function
> to avoid tripping up on that with CLang.
>
> New CLang has started having trouble with g_autoptr now too. In usage
> scenarios where the variable is set, but never again read, it thinks
> it is unused not realizing the destructor has useful side effects.
> For this we have to skip -Wunused-but-set-variable on CLang.
s/CLang/Clang/
> Signed-off-by: Daniel P. Berrangé
> ---
> meson.build | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 5af09d319a..dbd70b6483 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -470,6 +470,26 @@ if get_option('warning_level') == '2'
> supported_cc_flags += [ '-Wno-unused-function' ]
>endif
>
> + # Clang complains about unused variables in many scenarios arelated
s/arelated/related/
> + # to attribute((cleanup)) aka g_auto*
> + w_unused_but_set_var_args = [ '-Wunused-but-set-variable', '-Werror' ]
> + w_unused_but_set_var_code = '''
> +static inline void free_pointer (void *p) {
> + void **pp = (void**)p;
> + free (*pp);
> +}
> +
> +int main(void) {
> + __attribute__((cleanup(free_pointer))) char *buffer = 0x0;
> + buffer = 0x1;
> +}
> + '''
> + # We previously turned on unused-but-set-variable, so we must turn
> + # it off again explicitly now.
> + if not cc.compiles(w_unused_but_set_var_code, args:
> w_unused_but_set_var_args)
> +supported_cc_flags += [ '-Wno-unused-but-set-variable' ]
> + endif
> +
> endif
> add_project_arguments(supported_cc_flags, language: 'c')
Reviewed-by: Pavel Hrdina
signature.asc
Description: PGP signature
Re: [libvirt PATCH] tests: virstoragetest: remove tests without backing type
On Tue, Aug 31, 2021 at 12:16:13 +0100, Daniel P. Berrangé wrote: > On Tue, Aug 31, 2021 at 12:03:06PM +0200, Ján Tomko wrote: > > As of qemu commit: > > > > commit 497a30dbb065937d67f6c43af6dd78492e1d6f6d > > qemu-img: Require -F with -b backing image > > > > creating images with backing images requires specifying the format. > > > > Remove tests which do not pass the backing format on the command > > line. > > > > Signed-off-by: Ján Tomko > > --- > > tests/virstoragetest.c | 33 - > > 1 file changed, 33 deletions(-) > > This fixes the broken 'make check' on Fedora rawhide with QEMU 6.1.0 > > Reviewed-by: Daniel P. Berrangé Since I don't have time to come up with a proper fix for the release, please push this as-is for now and I'll revert it later. The cases being removed are important for testing the secure handling of backing images missing format so we'll need to have a stash of images if qemu-img can't create them any more. Note that I think the qemu patch forbidding format-less images is unfortunate because fixing them is not always secure and this can lead to users automating it in an insecure way.
Re: [libvirt PATCH] news: add FC VMID entry
On Tue, Aug 31, 2021 at 02:53:57PM +0200, Pavel Hrdina wrote: > + * Add support for Fibre Channel VMID > + > +New VM element was added to allow users set *allow users to set Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] meson: avoid bogus warnings from clang and g_autoptr
Clang has previously had trouble with G_DEFINE_AUTOPTR_CLEANUP_FUNC
generated code, thinking it was unused. We turn off -Wunused-function
to avoid tripping up on that with CLang.
New CLang has started having trouble with g_autoptr now too. In usage
scenarios where the variable is set, but never again read, it thinks
it is unused not realizing the destructor has useful side effects.
For this we have to skip -Wunused-but-set-variable on CLang.
Signed-off-by: Daniel P. Berrangé
---
meson.build | 20
1 file changed, 20 insertions(+)
diff --git a/meson.build b/meson.build
index 5af09d319a..dbd70b6483 100644
--- a/meson.build
+++ b/meson.build
@@ -470,6 +470,26 @@ if get_option('warning_level') == '2'
supported_cc_flags += [ '-Wno-unused-function' ]
endif
+ # Clang complains about unused variables in many scenarios arelated
+ # to attribute((cleanup)) aka g_auto*
+ w_unused_but_set_var_args = [ '-Wunused-but-set-variable', '-Werror' ]
+ w_unused_but_set_var_code = '''
+static inline void free_pointer (void *p) {
+ void **pp = (void**)p;
+ free (*pp);
+}
+
+int main(void) {
+ __attribute__((cleanup(free_pointer))) char *buffer = 0x0;
+ buffer = 0x1;
+}
+ '''
+ # We previously turned on unused-but-set-variable, so we must turn
+ # it off again explicitly now.
+ if not cc.compiles(w_unused_but_set_var_code, args:
w_unused_but_set_var_args)
+supported_cc_flags += [ '-Wno-unused-but-set-variable' ]
+ endif
+
endif
add_project_arguments(supported_cc_flags, language: 'c')
--
2.31.1
[PATCH 1/2] qemu: probe for virtio-blk-pci discard option support
Signed-off-by: yuxiating
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
27 files changed, 28 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 70c3ec2f0c..78ec31f807 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -638,6 +638,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"query-display-options", /* QEMU_CAPS_QUERY_DISPLAY_OPTIONS */
"s390-pv-guest", /* QEMU_CAPS_S390_PV_GUEST */
"set-action", /* QEMU_CAPS_SET_ACTION */
+ "virtio-blk-pci.discard", /* QEMU_CAPS_DEVICE_DISCARD */
);
@@ -1406,6 +1407,7 @@ static struct virQEMUCapsDevicePropsFlags
virQEMUCapsDevicePropsVirtioBlk[] = {
{ "werror", QEMU_CAPS_STORAGE_WERROR, NULL },
{ "packed", QEMU_CAPS_VIRTIO_PACKED_QUEUES, NULL },
{ "acpi-index", QEMU_CAPS_ACPI_INDEX, NULL },
+{ "discard", QEMU_CAPS_DEVICE_DISCARD, NULL },
};
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioNet[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bc762d1916..a05c98b71d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -618,6 +618,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command
present */
QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */
QEMU_CAPS_SET_ACTION, /* 'set-action' QMP command */
+QEMU_CAPS_DEVICE_DISCARD, /* -device virtio-blk-pci,discard=on/off */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
index efb891fa01..8603c8a78d 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
@@ -174,6 +174,7 @@
+
400
0
61700240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index 1e2b7c7fe6..67c7409176 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -181,6 +181,7 @@
+
400
0
42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
index 5872ecd491..ed906e4bb3 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
@@ -173,6 +173,7 @@
+
400
0
0
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
index bb76faae2b..66888f597b 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
@@ -173,6 +173,7 @@
+
400
0
0
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index 51074b4f37..66ad9021e1 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
@@ -137,6 +137,7 @@
+
400
0
39100240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
index 19b8a49394..d349e3e3c8 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
+++
[PATCH 2/2] qemu: support for virtio-blk-pci discard options
DISCARD and WRITE_ZEROES features for machine type >= 4.0 is enabled by default
since
commit 5c81161f804144b146607f890e84613a4cbad95c
virtio-blk: add "discard" and "write-zeroes" properties
Sometimes guestos has bugs DISCARD need to be disabled.
Signed-off-by: yuxiating
---
src/conf/domain_conf.c | 15 +++
src/conf/domain_conf.h | 9 +
src/conf/domain_validate.c | 6 ++
src/libvirt_private.syms | 3 ++-
src/qemu/qemu_command.c| 11 +++
5 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6127513117..bfe4721e60 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1278,6 +1278,13 @@ VIR_ENUM_IMPL(virDomainDiskDiscard,
"ignore",
);
+VIR_ENUM_IMPL(virDomainDiskDiscardEnable,
+ VIR_DOMAIN_DISK_DISCARD_ENABLE_LAST,
+ "default",
+ "off",
+ "on",
+);
+
VIR_ENUM_IMPL(virDomainDiskDetectZeroes,
VIR_DOMAIN_DISK_DETECT_ZEROES_LAST,
"default",
@@ -8930,6 +8937,10 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
if (virXMLPropUInt(cur, "queues", 10, VIR_XML_PROP_NONE, >queues) < 0)
return -1;
+if (virXMLPropEnum(cur, "discard_enable",
virDomainDiskDiscardEnableTypeFromString,
+ VIR_XML_PROP_NONZERO, >discard_enable) < 0)
+return -1;
+
return 0;
}
@@ -23416,6 +23427,10 @@ virDomainDiskDefFormatDriver(virBuffer *buf,
if (disk->queues)
virBufferAsprintf(, " queues='%u'", disk->queues);
+if (disk->discard_enable)
+virBufferAsprintf(, " discard_enable='%s'",
+
virDomainDiskDiscardEnableTypeToString(disk->discard_enable));
+
virDomainVirtioOptionsFormat(, disk->virtio);
if (disk->src->metadataCacheMaxSize > 0) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c7e6df7981..c39694a19e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -457,6 +457,13 @@ typedef enum {
VIR_DOMAIN_DISK_DISCARD_LAST
} virDomainDiskDiscard;
+typedef enum {
+VIR_DOMAIN_DISK_DISCARD_ENABLE_DEFAULT = 0,
+VIR_DOMAIN_DISK_DISCARD_ENABLE_OFF,
+VIR_DOMAIN_DISK_DISCARD_ENABLE_ON,
+VIR_DOMAIN_DISK_DISCARD_ENABLE_LAST
+} virDomainDiskDiscardEnable;
+
typedef enum {
VIR_DOMAIN_DISK_DETECT_ZEROES_DEFAULT = 0,
VIR_DOMAIN_DISK_DETECT_ZEROES_OFF,
@@ -589,6 +596,7 @@ struct _virDomainDiskDef {
bool diskElementAuth;
bool diskElementEnc;
+virDomainDiskDiscardEnable discard_enable;
};
@@ -3838,6 +3846,7 @@ VIR_ENUM_DECL(virDomainDiskIo);
VIR_ENUM_DECL(virDomainDeviceSGIO);
VIR_ENUM_DECL(virDomainDiskTray);
VIR_ENUM_DECL(virDomainDiskDiscard);
+VIR_ENUM_DECL(virDomainDiskDiscardEnable);
VIR_ENUM_DECL(virDomainDiskDetectZeroes);
VIR_ENUM_DECL(virDomainDiskModel);
VIR_ENUM_DECL(virDomainDiskMirrorState);
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 60f7cc..6eb346916a 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -612,6 +612,12 @@ virDomainDiskDefValidate(const virDomainDef *def,
return -1;
}
+if (disk->discard_enable) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("discard_enable attribute in disk driver element
is only supported by virtio-blk"));
+return -1;
+}
+
if (disk->event_idx != VIR_TRISTATE_SWITCH_ABSENT) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("disk event_idx mode supported only for virtio
bus"));
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ab8a6c00c3..52a74dd2d5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1,5 +1,5 @@
-#
# General private symbols. Add symbols here, and see src/meson.build for
+# mainDiskDeviceTypeToString
# more details.
#
# Keep this file sorted by header name, then by symbols with each header.
@@ -377,6 +377,7 @@ virDomainDiskDefParseSource;
virDomainDiskDetectZeroesTypeFromString;
virDomainDiskDetectZeroesTypeToString;
virDomainDiskDeviceTypeToString;
+virDomainDiskDiscardEnableTypeToString;
virDomainDiskDiscardTypeToString;
virDomainDiskEmptySource;
virDomainDiskErrorPolicyTypeFromString;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b230314f7f..894c8b17b9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1739,6 +1739,17 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
virBufferAsprintf(, ",num-queues=%u", disk->queues);
}
+if (disk->discard_enable) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DISCARD)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio-blk discard property isn't supported
by this "
+
[libvirt PATCH] news: add FC VMID entry
Signed-off-by: Pavel Hrdina --- NEWS.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 200256c299..72b63a19ea 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,13 @@ v7.7.0 (unreleased) * **New features** + * Add support for Fibre Channel VMID + +New VM element was added to allow users set +their ``appid`` for each VM which will be used by kernel to create Fibre +Channel VMID. This allows various QoS levels, access control or collecting +telemetry data per VM. + * **Improvements** * virsh: Allow XML validation for define of: storage pool, network, secret, -- 2.31.1
Re: [libvirt PATCH] qemu, xen: add missing deps on virtlockd/virtlogd sockets
On Tue, Aug 31, 2021 at 12:04:14 +0100, Daniel P. Berrangé wrote: > The QEMU driver uses both virtlogd and virtlockd, while the Xen driver > uses virtlockd. The libvirtd.service unit contains deps on the socket > units for these services, but these deps were missed in the modular > daemons. As a result the virtlockd/virtlogd sockets are not started > when the virtqemud/virtxend daemons are started. > > Signed-off-by: Daniel P. Berrangé > --- > src/libxl/virtxend.service.in | 2 ++ > src/qemu/virtqemud.service.in | 4 > 2 files changed, 6 insertions(+) Reviewed-by: Jiri Denemark
Re: [libvirt PATCH] rpm: fix typo in post transaction scriptlet name
On Tue, Aug 31, 2021 at 12:03:33 +0100, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > libvirt.spec.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libvirt.spec.in b/libvirt.spec.in > index b01379d242..624b0e0302 100644 > --- a/libvirt.spec.in > +++ b/libvirt.spec.in > @@ -1441,7 +1441,7 @@ fi > %preun daemon-driver-secret > %libvirt_daemon_systemd_preun virtsecretd > > -%posttranstrans daemon-driver-secret > +%posttrans daemon-driver-secret > %libvirt_daemon_perform_restart virtsecretd Reviewed-by: Jiri Denemark
Re: [libvirt PATCH] rpm: fix typo in post transaction scriptlet name
On Tue, Aug 31, 2021 at 12:03:33PM +0100, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > libvirt.spec.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [libvirt PATCH] tests: virstoragetest: remove tests without backing type
On Tue, Aug 31, 2021 at 12:03:06PM +0200, Ján Tomko wrote: > As of qemu commit: > > commit 497a30dbb065937d67f6c43af6dd78492e1d6f6d > qemu-img: Require -F with -b backing image > > creating images with backing images requires specifying the format. > > Remove tests which do not pass the backing format on the command > line. > > Signed-off-by: Ján Tomko > --- > tests/virstoragetest.c | 33 - > 1 file changed, 33 deletions(-) This fixes the broken 'make check' on Fedora rawhide with QEMU 6.1.0 Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[libvirt PATCH] qemu, xen: add missing deps on virtlockd/virtlogd sockets
The QEMU driver uses both virtlogd and virtlockd, while the Xen driver uses virtlockd. The libvirtd.service unit contains deps on the socket units for these services, but these deps were missed in the modular daemons. As a result the virtlockd/virtlogd sockets are not started when the virtqemud/virtxend daemons are started. Signed-off-by: Daniel P. Berrangé --- src/libxl/virtxend.service.in | 2 ++ src/qemu/virtqemud.service.in | 4 2 files changed, 6 insertions(+) diff --git a/src/libxl/virtxend.service.in b/src/libxl/virtxend.service.in index a863917467..19b19ce3e6 100644 --- a/src/libxl/virtxend.service.in +++ b/src/libxl/virtxend.service.in @@ -1,6 +1,7 @@ [Unit] Description=Virtualization xen daemon Conflicts=libvirtd.service +Requires=virtlockd.socket Requires=virtxend.socket Requires=virtxend-ro.socket Requires=virtxend-admin.socket @@ -25,6 +26,7 @@ KillMode=process [Install] WantedBy=multi-user.target +Also=virtlockd.socket Also=virtxend.socket Also=virtxend-ro.socket Also=virtxend-admin.socket diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in index 8abc9d3a7f..20e1b43a6e 100644 --- a/src/qemu/virtqemud.service.in +++ b/src/qemu/virtqemud.service.in @@ -1,6 +1,8 @@ [Unit] Description=Virtualization qemu daemon Conflicts=libvirtd.service +Requires=virtlogd.socket +Requires=virtlockd.socket Requires=virtqemud.socket Requires=virtqemud-ro.socket Requires=virtqemud-admin.socket @@ -42,6 +44,8 @@ LimitMEMLOCK=64M [Install] WantedBy=multi-user.target +Also=virtlogd.socket +Also=virtlockd.socket Also=virtqemud.socket Also=virtqemud-ro.socket Also=virtqemud-admin.socket -- 2.31.1
[libvirt PATCH] rpm: fix typo in post transaction scriptlet name
Signed-off-by: Daniel P. Berrangé --- libvirt.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index b01379d242..624b0e0302 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1441,7 +1441,7 @@ fi %preun daemon-driver-secret %libvirt_daemon_systemd_preun virtsecretd -%posttranstrans daemon-driver-secret +%posttrans daemon-driver-secret %libvirt_daemon_perform_restart virtsecretd -- 2.31.1
[PATCH 1/1] qemu: add virtio-blk queue-size option
The option "queue-size" in virtio-blk was added in qemu-2.12.0, and default
value increased from qemu-5.0.0.
However, increasing this value may lead to drop of random access performance.
This is configurable value, so we want to use it via libvirt.
Signed-off-by: Hiroki Narukawa
---
docs/schemas/domaincommon.rng | 5 +++
src/conf/domain_conf.c| 6
src/conf/domain_conf.h| 1 +
src/qemu/qemu_capabilities.c | 5 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 3 ++
src/qemu/qemu_validate.c | 7
.../caps_2.12.0.aarch64.xml | 1 +
.../caps_2.12.0.s390x.xml | 1 +
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 +
.../caps_3.0.0.riscv32.xml| 1 +
.../caps_3.0.0.riscv64.xml| 1 +
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 +
.../caps_3.0.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 +
.../caps_3.1.0.x86_64.xml | 1 +
.../caps_4.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
.../caps_4.0.0.riscv32.xml| 1 +
.../caps_4.0.0.riscv64.xml| 1 +
.../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
.../caps_4.0.0.x86_64.xml | 1 +
.../caps_4.1.0.x86_64.xml | 1 +
.../caps_4.2.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
.../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
.../caps_4.2.0.x86_64.xml | 1 +
.../caps_5.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
.../caps_5.0.0.riscv64.xml| 1 +
.../caps_5.0.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
.../caps_5.1.0.x86_64.xml | 1 +
.../caps_5.2.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
.../caps_5.2.0.riscv64.xml| 1 +
.../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
.../caps_5.2.0.x86_64.xml | 1 +
.../caps_6.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 +
.../caps_6.0.0.x86_64.xml | 1 +
.../caps_6.1.0.x86_64.xml | 1 +
.../disk-virtio-queue-size.args | 29 +++
.../disk-virtio-queue-size.xml| 35 +++
tests/qemuxml2argvtest.c | 2 ++
.../disk-virtio-queue-size.xml| 35 +++
tests/qemuxml2xmltest.c | 1 +
47 files changed, 165 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.args
create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.xml
create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-queue-size.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 11fa24f398..fdc04f90aa 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2363,6 +2363,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6127513117..cfce32379e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8930,6 +8930,9 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
if (virXMLPropUInt(cur, "queues", 10, VIR_XML_PROP_NONE, >queues) < 0)
return -1;
+if (virXMLPropUInt(cur, "queue_size", 10, VIR_XML_PROP_NONE,
>queue_size) < 0)
+return -1;
+
return 0;
}
@@ -23416,6 +23419,9 @@ virDomainDiskDefFormatDriver(virBuffer *buf,
if (disk->queues)
virBufferAsprintf(, " queues='%u'", disk->queues);
+if (disk->queue_size)
+virBufferAsprintf(, " queue_size='%u'", disk->queue_size);
+
virDomainVirtioOptionsFormat(, disk->virtio);
if (disk->src->metadataCacheMaxSize > 0) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c7e6df7981..688a842660 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -584,6 +584,7 @@ struct _virDomainDiskDef {
virDomainDiskDetectZeroes detect_zeroes;
char *domain_name; /* backend domain name */
unsigned int queues;
+unsigned int queue_size;
virDomainDiskModel model;
virDomainVirtioOptions *virtio;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 70c3ec2f0c..ee59e8e961 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -638,6 +638,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"query-display-options", /* QEMU_CAPS_QUERY_DISPLAY_OPTIONS */
[PATCH 0/1] qemu: add virtio-blk queue-size option
Hello, The option "queue-size" in virtio-blk was added in qemu-2.12.0, and default value increased from qemu-5.0.0. However, increasing this value may lead to drop of random access performance. This is configurable value, so we want to use it via libvirt. Sorry, my previous e-mail had wrong CC address that is rejected, please reply to this message and not previous one. Could you review this patch? Hiroki Narukawa (1): qemu: add virtio-blk queue-size option docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c| 6 src/conf/domain_conf.h| 1 + src/qemu/qemu_capabilities.c | 5 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 3 ++ src/qemu/qemu_validate.c | 7 .../caps_2.12.0.aarch64.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + .../caps_3.0.0.riscv32.xml| 1 + .../caps_3.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 + .../caps_3.1.0.x86_64.xml | 1 + .../caps_4.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml| 1 + .../caps_4.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml| 1 + .../caps_5.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + .../disk-virtio-queue-size.args | 29 +++ .../disk-virtio-queue-size.xml| 35 +++ tests/qemuxml2argvtest.c | 2 ++ .../disk-virtio-queue-size.xml| 35 +++ tests/qemuxml2xmltest.c | 1 + 47 files changed, 165 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-queue-size.xml -- 2.17.1
[PATCH 0/1] qemu: add virtio-blk queue-size option
Hello, The option "queue-size" in virtio-blk was added in qemu-2.12.0, and default value increased from qemu-5.0.0. However, increasing this value may lead to drop of random access performance. This is configurable value, so we want to use it via libvirt. Could you review this patch? Hiroki Narukawa (1): qemu: add virtio-blk queue-size option docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c| 6 src/conf/domain_conf.h| 1 + src/qemu/qemu_capabilities.c | 5 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 3 ++ src/qemu/qemu_validate.c | 7 .../caps_2.12.0.aarch64.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + .../caps_3.0.0.riscv32.xml| 1 + .../caps_3.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 + .../caps_3.1.0.x86_64.xml | 1 + .../caps_4.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml| 1 + .../caps_4.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml| 1 + .../caps_5.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + .../disk-virtio-queue-size.args | 29 +++ .../disk-virtio-queue-size.xml| 35 +++ tests/qemuxml2argvtest.c | 2 ++ .../disk-virtio-queue-size.xml| 35 +++ tests/qemuxml2xmltest.c | 1 + 47 files changed, 165 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-queue-size.xml -- 2.17.1
[libvirt PATCH] tests: virstoragetest: remove tests without backing type
As of qemu commit:
commit 497a30dbb065937d67f6c43af6dd78492e1d6f6d
qemu-img: Require -F with -b backing image
creating images with backing images requires specifying the format.
Remove tests which do not pass the backing format on the command
line.
Signed-off-by: Ján Tomko
---
tests/virstoragetest.c | 33 -
1 file changed, 33 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 1b211b60e6..b80818bc7b 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -638,30 +638,6 @@ mymain(void)
};
TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (, , ),
EXP_PASS);
-/* Rewrite qcow2 and wrap file to omit backing file type */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-b", absraw, "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-b", absqcow2, "wrap", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
-/* Qcow2 file with raw as absolute backing, backing format omitted */
-testFileData wrap_as_raw = {
-.expBackingStoreRaw = absqcow2,
-.expCapacity = 1024,
-.path = abswrap,
-.type = VIR_STORAGE_TYPE_FILE,
-.format = VIR_STORAGE_FILE_QCOW2,
-};
-TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2,
- (_as_raw, _as_raw), EXP_FAIL);
-
/* Rewrite qcow2 to a missing backing file, with backing type */
virCommandFree(cmd);
cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
@@ -674,15 +650,6 @@ mymain(void)
/* Qcow2 file with missing backing file but specified type */
TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (), EXP_FAIL);
-/* Rewrite qcow2 to a missing backing file, without backing type */
-virCommandFree(cmd);
-cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2",
- "-b", datadir "/bogus", "qcow2", NULL);
-if (virCommandRun(cmd, NULL) < 0)
-ret = -1;
-
-/* Qcow2 file with missing backing file and no specified type */
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (), EXP_FAIL);
/* Rewrite qcow2 to use an nbd: protocol as backend */
virCommandFree(cmd);
--
2.31.1
[PATCH 1/1] qemu: add virtio-blk queue-size option
The option "queue-size" in virtio-blk was added in qemu-2.12.0, and default
value increased from qemu-5.0.0.
However, increasing this value may lead to drop of random access performance.
This is configurable value, so we want to use it via libvirt.
Signed-off-by: Hiroki Narukawa
---
docs/schemas/domaincommon.rng | 5 +++
src/conf/domain_conf.c| 6
src/conf/domain_conf.h| 1 +
src/qemu/qemu_capabilities.c | 5 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 3 ++
src/qemu/qemu_validate.c | 7
.../caps_2.12.0.aarch64.xml | 1 +
.../caps_2.12.0.s390x.xml | 1 +
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 +
.../caps_3.0.0.riscv32.xml| 1 +
.../caps_3.0.0.riscv64.xml| 1 +
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 +
.../caps_3.0.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 +
.../caps_3.1.0.x86_64.xml | 1 +
.../caps_4.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
.../caps_4.0.0.riscv32.xml| 1 +
.../caps_4.0.0.riscv64.xml| 1 +
.../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
.../caps_4.0.0.x86_64.xml | 1 +
.../caps_4.1.0.x86_64.xml | 1 +
.../caps_4.2.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
.../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
.../caps_4.2.0.x86_64.xml | 1 +
.../caps_5.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
.../caps_5.0.0.riscv64.xml| 1 +
.../caps_5.0.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
.../caps_5.1.0.x86_64.xml | 1 +
.../caps_5.2.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
.../caps_5.2.0.riscv64.xml| 1 +
.../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
.../caps_5.2.0.x86_64.xml | 1 +
.../caps_6.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 +
.../caps_6.0.0.x86_64.xml | 1 +
.../caps_6.1.0.x86_64.xml | 1 +
.../disk-virtio-queue-size.args | 29 +++
.../disk-virtio-queue-size.xml| 35 +++
tests/qemuxml2argvtest.c | 2 ++
.../disk-virtio-queue-size.xml| 35 +++
tests/qemuxml2xmltest.c | 1 +
47 files changed, 165 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.args
create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.xml
create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-queue-size.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 11fa24f398..fdc04f90aa 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2363,6 +2363,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6127513117..cfce32379e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8930,6 +8930,9 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
if (virXMLPropUInt(cur, "queues", 10, VIR_XML_PROP_NONE, >queues) < 0)
return -1;
+if (virXMLPropUInt(cur, "queue_size", 10, VIR_XML_PROP_NONE,
>queue_size) < 0)
+return -1;
+
return 0;
}
@@ -23416,6 +23419,9 @@ virDomainDiskDefFormatDriver(virBuffer *buf,
if (disk->queues)
virBufferAsprintf(, " queues='%u'", disk->queues);
+if (disk->queue_size)
+virBufferAsprintf(, " queue_size='%u'", disk->queue_size);
+
virDomainVirtioOptionsFormat(, disk->virtio);
if (disk->src->metadataCacheMaxSize > 0) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c7e6df7981..688a842660 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -584,6 +584,7 @@ struct _virDomainDiskDef {
virDomainDiskDetectZeroes detect_zeroes;
char *domain_name; /* backend domain name */
unsigned int queues;
+unsigned int queue_size;
virDomainDiskModel model;
virDomainVirtioOptions *virtio;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 70c3ec2f0c..ee59e8e961 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -638,6 +638,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"query-display-options", /* QEMU_CAPS_QUERY_DISPLAY_OPTIONS */
Re: [PATCH v2 2/2] qemuValidateDomainDeviceDefDiskTransient: Add checking set-action capability
On Mon, Aug 30, 2021 at 00:30:43 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma
>
> option requires set-action capability
> if destroy action is set to all of the events configuration.
>
> Signed-off-by: Masayoshi Mizuma
> ---
> src/qemu/qemu_validate.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 8906aa52d9..0d49512996 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
[...]
> @@ -2991,6 +2992,7 @@ qemuValidateDomainDeviceDefDiskBlkdeviotune(const
> virDomainDiskDef *disk,
>
> static int
> qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk,
> + const virDomainDef *def,
> virQEMUCaps *qemuCaps)
>
> {
> @@ -3033,6 +3035,15 @@ qemuValidateDomainDeviceDefDiskTransient(const
> virDomainDiskDef *disk,
> }
>
> if (disk->transientShareBacking == VIR_TRISTATE_BOOL_YES) {
> +if (!qemuProcessRebootAllowed(def) &&
> +!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SET_ACTION)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("transient disk backing image sharing with
> destroy action of lifecycle "
> + "isn't supported by this QEMU binary (%s)"),
> + disk->dst);
> +return -1;
I didn't really consider this corner case worth worrying about.
I'll exchange the two terms in the condition since I prefer when
capability checks go first in such cases and put the error message on a
single line for easier greppability and also add a better explanation of
the specific scenario this is handling into the commit message.
Series:
Reviewed-by: Peter Krempa
Re: [PATCH v2 1/2] qemu: process: Split out the statement to handle the qemu is allowed to reboot
On Mon, Aug 30, 2021 at 00:30:42 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma
>
> Split out the statement to handle whether the qemu is allowed to reboot
> or not. So that it gets available for the later patch.
>
> Signed-off-by: Masayoshi Mizuma
> ---
> src/qemu/qemu_process.c | 17 +
> src/qemu/qemu_process.h | 2 ++
> 2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 3b4af61bf8..f4e67c70ad 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6360,6 +6360,18 @@ qemuProcessPrepareHostHostdevs(virDomainObj *vm)
> return 0;
> }
>
> +bool
> +qemuProcessRebootAllowed(const virDomainDef *def)
> +{
> +if (def->onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY &&
> +def->onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY &&
> +(def->onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY ||
> + def->onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY)) {
> +return false;
> +} else {
> +return true;
> +}
> +}
if (COND)
return true/false;
else
return false/true;
is an anti-pattern. The value can be directly returned. In addition it's
missing a function comment when this is actually used since you are
moving it from the place which is doing the feature check.
>
> static void
> qemuProcessPrepareAllowReboot(virDomainObj *vm)
> @@ -6375,10 +6387,7 @@ qemuProcessPrepareAllowReboot(virDomainObj *vm)
> if (priv->allowReboot != VIR_TRISTATE_BOOL_ABSENT)
> return;
>
> -if (def->onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY &&
> -def->onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY &&
> -(def->onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY ||
> - def->onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY)) {
> +if (!qemuProcessRebootAllowed(def)) {
> priv->allowReboot = VIR_TRISTATE_BOOL_NO;
> } else {
> priv->allowReboot = VIR_TRISTATE_BOOL_YES;
Here we can use virTristateBoolFromBool instead of an if statement.
I'll go with:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3b4af61bf8..03915f559c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6361,6 +6361,24 @@ qemuProcessPrepareHostHostdevs(virDomainObj *vm)
}
+/**
+ * qemuProcessRebootAllowed:
+ * @def: domain definition
+ *
+ * This function encapsulates the logic which dictated whether '-no-reboot' was
+ * used instead of '-no-shutdown' which is used QEMU versions which don't
+ * support the 'set-action' QMP command.
+ */
+bool
+qemuProcessRebootAllowed(const virDomainDef *def)
+{
+return def->onReboot != VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY ||
+ def->onPoweroff != VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY ||
+ (def->onCrash != VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY &&
+def->onCrash != VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY);
+}
+
+
static void
qemuProcessPrepareAllowReboot(virDomainObj *vm)
{
@@ -6375,14 +6393,7 @@ qemuProcessPrepareAllowReboot(virDomainObj *vm)
if (priv->allowReboot != VIR_TRISTATE_BOOL_ABSENT)
return;
-if (def->onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY &&
-def->onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY &&
-(def->onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY ||
- def->onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY)) {
-priv->allowReboot = VIR_TRISTATE_BOOL_NO;
-} else {
-priv->allowReboot = VIR_TRISTATE_BOOL_YES;
-}
+priv->allowReboot = virTristateBoolFromBool(qemuProcessRebootAllowed(def));
}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 93103eb530..f9fa140e6d 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -242,3 +242,5 @@ void qemuProcessQMPFree(qemuProcessQMP *proc);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuProcessQMP, qemuProcessQMPFree);
int qemuProcessQMPStart(qemuProcessQMP *proc);
+
+bool qemuProcessRebootAllowed(const virDomainDef *def);
Re: [PATCH] qemuProcessSetupDisksTransientHotplug: Add checking set-action capability
On Wed, Aug 25, 2021 at 19:08:46 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma
>
> The VM is terminated abnormally when
> is set to the disk option and the qemu doesn't have set-action capability.
>
> # virsh start guest01
> error: Failed to start domain 'guest01'
> error: internal error: qemu unexpectedly closed the monitor
>
> #
>
> Add checking the capability before system_reset QMP command is sent
> so that the VM can stop correctly when the qemu doesn't have the cap.
>
> Signed-off-by: Masayoshi Mizuma
> ---
> src/qemu/qemu_process.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 3b4af61bf8..4bedd04679 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7010,6 +7010,9 @@ qemuProcessSetupDisksTransientHotplug(virDomainObj *vm,
> if (hasHotpluggedDisk) {
> int rc;
>
> +if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)))
> +return -1;
This should be in one of the validation functions checking support for
transient disks and with an appropriate error message.
