Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions
in libvrit-domain.c
by m kamal
After seeing a couple of patches in the list, it looks like [PATCH v2] is the usual expected behavior. So, I will proceed with that.
Many thanks. Sorry for any inconvenience.
________________________________
From: m kamal <recenum(a)outlook.com>
Sent: Monday, March 11, 2024 3:45 PM
To: Daniel P. Berrangé <berrange(a)redhat.com>
Cc: devel(a)lists.libvirt.org <devel(a)lists.libvirt.org>; pkrempa(a)redhat.com <pkrempa(a)redhat.com>
Subject: Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Hello Daniel, Peter,
Apologies for the accidental noreply mail in the CC, it's probably the "supresscc = self" line that I copied blindly into my send-email git config. Also, many apologies for forgetting to at least compile the change. I have read the guidelines before but I forgot.
Question: After I amend the commit with the right changes, should I simply call "git publish" again?
I tested this in a small repo I made and the behaviour is that "git publish" starts a new email chain with [PATCH v2]. Is this what should happen or is it required for the new PATCH to be a reply to this chain?
Cheers,
Mostafa
________________________________
From: Daniel P. Berrangé <berrange(a)redhat.com>
Sent: Monday, March 11, 2024 9:24 AM
To: Mostafa <recenum(a)outlook.com>
Cc: devel(a)lists.libvirt.org <devel(a)lists.libvirt.org>
Subject: Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
On Mon, Mar 11, 2024 at 02:15:32AM +0200, Mostafa wrote:
> From: مصطفي محمود كمال الدين <48567303+moste00(a)users.noreply.github.com>
>
> ---
> src/libvirt-domain.c | 32 ++++++++------------------------
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 83abad251e..9b68a7ac95 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> if (conn->driver->domainSave) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
All variables declared with 'g_autofree' *must* be initialized
at time of declaration, so you need to add ' = NULL' here and
to all the other similar changes.
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> ret = conn->driver->domainSave(domain, absolute_to);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> if (conn->driver->domainSaveFlags) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml, flags);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> if (conn->driver->domainRestore) {
> int ret;
> - char *absolute_from;
> + g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> ret = conn->driver->domainRestore(conn, absolute_from);
>
> - VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
>
> if (conn->driver->domainRestoreFlags) {
> int ret;
> - char *absolute_from;
> + g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
> ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
> flags);
>
> - VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
>
> if (conn->driver->domainSaveImageGetXMLDesc) {
> char *ret;
> - char *absolute_file;
> + g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1275,8 +1267,6 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
> ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file,
> flags);
>
> - VIR_FREE(absolute_file);
> -
> if (!ret)
> goto error;
> return ret;
> @@ -1338,7 +1328,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
>
> if (conn->driver->domainSaveImageDefineXML) {
> int ret;
> - char *absolute_file;
> + g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1350,8 +1340,6 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
> ret = conn->driver->domainSaveImageDefineXML(conn, absolute_file,
> dxml, flags);
>
> - VIR_FREE(absolute_file);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1415,7 +1403,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
>
> if (conn->driver->domainCoreDump) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -1426,8 +1414,6 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
>
> ret = conn->driver->domainCoreDump(domain, absolute_to, flags);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1501,7 +1487,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
>
> if (conn->driver->domainCoreDumpWithFormat) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -1513,8 +1499,6 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
> ret = conn->driver->domainCoreDumpWithFormat(domain, absolute_to,
> dumpformat, flags);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> --
> 2.34.1
> _______________________________________________
> Devel mailing list -- devel(a)lists.libvirt.org
> To unsubscribe send an email to devel-leave(a)lists.libvirt.org
With 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 :|
8 months, 2 weeks
Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions
in libvrit-domain.c
by Daniel P. Berrangé
On Mon, Mar 11, 2024 at 02:15:32AM +0200, Mostafa wrote:
> From: مصطفي محمود كمال الدين <48567303+moste00(a)users.noreply.github.com>
>
> ---
> src/libvirt-domain.c | 32 ++++++++------------------------
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 83abad251e..9b68a7ac95 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> if (conn->driver->domainSave) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
All variables declared with 'g_autofree' *must* be initialized
at time of declaration, so you need to add ' = NULL' here and
to all the other similar changes.
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> ret = conn->driver->domainSave(domain, absolute_to);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> if (conn->driver->domainSaveFlags) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml, flags);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> if (conn->driver->domainRestore) {
> int ret;
> - char *absolute_from;
> + g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> ret = conn->driver->domainRestore(conn, absolute_from);
>
> - VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
>
> if (conn->driver->domainRestoreFlags) {
> int ret;
> - char *absolute_from;
> + g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
> ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
> flags);
>
> - VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
>
> if (conn->driver->domainSaveImageGetXMLDesc) {
> char *ret;
> - char *absolute_file;
> + g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1275,8 +1267,6 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
> ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file,
> flags);
>
> - VIR_FREE(absolute_file);
> -
> if (!ret)
> goto error;
> return ret;
> @@ -1338,7 +1328,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
>
> if (conn->driver->domainSaveImageDefineXML) {
> int ret;
> - char *absolute_file;
> + g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1350,8 +1340,6 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
> ret = conn->driver->domainSaveImageDefineXML(conn, absolute_file,
> dxml, flags);
>
> - VIR_FREE(absolute_file);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1415,7 +1403,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
>
> if (conn->driver->domainCoreDump) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -1426,8 +1414,6 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
>
> ret = conn->driver->domainCoreDump(domain, absolute_to, flags);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1501,7 +1487,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
>
> if (conn->driver->domainCoreDumpWithFormat) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -1513,8 +1499,6 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
> ret = conn->driver->domainCoreDumpWithFormat(domain, absolute_to,
> dumpformat, flags);
>
> - VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> --
> 2.34.1
> _______________________________________________
> Devel mailing list -- devel(a)lists.libvirt.org
> To unsubscribe send an email to devel-leave(a)lists.libvirt.org
With 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 :|
8 months, 2 weeks
Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions
in libvrit-domain.c
by Peter Krempa
Note that I had to drop the 'noreply.github.com' email address. Please
don't use that as the author credential as it makes hard to reply
properly to threads.
On Mon, Mar 11, 2024 at 02:15:32 +0200, Mostafa wrote:
> From: مصطفي محمود كمال الدين <48567303+moste00(a)users.noreply.github.com>
Per contributor guidelines:
Contributors to libvirt projects must assert that they are in
compliance with the Developer Certificate of Origin 1.1. This is
achieved by adding a "Signed-off-by" line containing the contributor's
name and e-mail to every commit message. The presence of this line
attests that the contributor has read the above lined DCO and agrees
with its statements.
https://www.libvirt.org/hacking.html#developer-certificate-of-origin
Please make sure to follow all points of that document, especially all
paragraphs about:
https://www.libvirt.org/hacking.html#preparing-patches
in future postings.
> ---
> src/libvirt-domain.c | 32 ++++++++------------------------
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 83abad251e..9b68a7ac95 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> if (conn->driver->domainSave) {
> int ret;
> - char *absolute_to;
> + g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
After this patch the code fails to compile:
In function ‘g_autoptr_cleanup_generic_gfree’,
inlined from ‘virDomainSave’ at ../../../libvirt/src/libvirt-domain.c:887:2 :
/usr/include/glib-2.0/glib/glib-autocleanups.h:30:3: error: ‘absolute_to’ may be used uninitialized [-Werror=maybe-uninitialized]
30 | g_free (*pp);
| ^~~~~~~~~~~~
../../../libvirt/src/libvirt-domain.c: In function ‘virDomainSave’:
../../../libvirt/src/libvirt-domain.c:887:26: note: ‘absolute_to’ was declared here
887 | g_autofree char *absolute_to;
| ^~~~~~~~~~~
In function ‘g_autoptr_cleanup_generic_gfree’,
inlined from ‘virDomainSaveFlags’ at ../../../libvirt/src/libvirt-domain.c:975:26:
/usr/include/glib-2.0/glib/glib-autocleanups.h:30:3: error: ‘absolute_to’ may be used uninitialized [-Werror=maybe-uninitialized]
30 | g_free (*pp);
| ^~~~~~~~~~~~
When compiling the code by default from a git checkout we enable all
errors thus you should be getting this error or the syntax-check error
for the same thing if you have an older compiler. Make sure to both
compile and run tests as the contributor guidelines state.
8 months, 2 weeks
[PATCH] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
by Mostafa
From: مصطفي محمود كمال الدين <48567303+moste00(a)users.noreply.github.com>
---
src/libvirt-domain.c | 32 ++++++++------------------------
1 file changed, 8 insertions(+), 24 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 83abad251e..9b68a7ac95 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
if (conn->driver->domainSave) {
int ret;
- char *absolute_to;
+ g_autofree char *absolute_to;
/* We must absolutize the file path as the save is done out of process */
if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
ret = conn->driver->domainSave(domain, absolute_to);
- VIR_FREE(absolute_to);
-
if (ret < 0)
goto error;
return ret;
@@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
if (conn->driver->domainSaveFlags) {
int ret;
- char *absolute_to;
+ g_autofree char *absolute_to;
/* We must absolutize the file path as the save is done out of process */
if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml, flags);
- VIR_FREE(absolute_to);
-
if (ret < 0)
goto error;
return ret;
@@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
if (conn->driver->domainRestore) {
int ret;
- char *absolute_from;
+ g_autofree char *absolute_from;
/* We must absolutize the file path as the restore is done out of process */
if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
@@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
ret = conn->driver->domainRestore(conn, absolute_from);
- VIR_FREE(absolute_from);
-
if (ret < 0)
goto error;
return ret;
@@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
if (conn->driver->domainRestoreFlags) {
int ret;
- char *absolute_from;
+ g_autofree char *absolute_from;
/* We must absolutize the file path as the restore is done out of process */
if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
@@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
flags);
- VIR_FREE(absolute_from);
-
if (ret < 0)
goto error;
return ret;
@@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
if (conn->driver->domainSaveImageGetXMLDesc) {
char *ret;
- char *absolute_file;
+ g_autofree char *absolute_file;
/* We must absolutize the file path as the read is done out of process */
if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
@@ -1275,8 +1267,6 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file,
flags);
- VIR_FREE(absolute_file);
-
if (!ret)
goto error;
return ret;
@@ -1338,7 +1328,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
if (conn->driver->domainSaveImageDefineXML) {
int ret;
- char *absolute_file;
+ g_autofree char *absolute_file;
/* We must absolutize the file path as the read is done out of process */
if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
@@ -1350,8 +1340,6 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
ret = conn->driver->domainSaveImageDefineXML(conn, absolute_file,
dxml, flags);
- VIR_FREE(absolute_file);
-
if (ret < 0)
goto error;
return ret;
@@ -1415,7 +1403,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
if (conn->driver->domainCoreDump) {
int ret;
- char *absolute_to;
+ g_autofree char *absolute_to;
/* We must absolutize the file path as the save is done out of process */
if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -1426,8 +1414,6 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
ret = conn->driver->domainCoreDump(domain, absolute_to, flags);
- VIR_FREE(absolute_to);
-
if (ret < 0)
goto error;
return ret;
@@ -1501,7 +1487,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
if (conn->driver->domainCoreDumpWithFormat) {
int ret;
- char *absolute_to;
+ g_autofree char *absolute_to;
/* We must absolutize the file path as the save is done out of process */
if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -1513,8 +1499,6 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
ret = conn->driver->domainCoreDumpWithFormat(domain, absolute_to,
dumpformat, flags);
- VIR_FREE(absolute_to);
-
if (ret < 0)
goto error;
return ret;
--
2.34.1
8 months, 2 weeks
[PULL 31/43] hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP configurations
by Philippe Mathieu-Daudé
From: Zhao Liu <zhao1.liu(a)intel.com>
Currently, it was allowed for users to specify the unsupported
topology parameter as "1". For example, x86 PC machine doesn't
support drawer/book/cluster topology levels, but user could specify
"-smp drawers=1,books=1,clusters=1".
This is meaningless and confusing, so that the support for this kind of
configurations is marked deprecated since 9.0. And report warning
message for such case like:
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported clusters parameter mustn't be specified as 1
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported books parameter mustn't be specified as 1
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported drawers parameter mustn't be specified as 1
Users have to ensure that all the topology members described with -smp
are supported by the target machine.
Signed-off-by: Zhao Liu <zhao1.liu(a)intel.com>
Reviewed-by: Thomas Huth <thuth(a)redhat.com>
Message-ID: <20240308160148.3130837-3-zhao1.liu(a)linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
---
docs/about/deprecated.rst | 14 +++++++++
hw/core/machine-smp.c | 65 +++++++++++++++++++++++++++++----------
2 files changed, 62 insertions(+), 17 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 6e2f557682..dfd681cd02 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -57,6 +57,20 @@ The ``-p`` option pretends to control the host page size. However,
it is not possible to change the host page size, and using the
option only causes failures.
+``-smp`` (Unsupported "parameter=1" SMP configurations) (since 9.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Specified CPU topology parameters must be supported by the machine.
+
+In the SMP configuration, users should provide the CPU topology parameters that
+are supported by the target machine.
+
+However, historically it was allowed for users to specify the unsupported
+topology parameter as "1", which is meaningless. So support for this kind of
+configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is
+marked deprecated since 9.0, users have to ensure that all the topology members
+described with -smp are supported by the target machine.
+
QEMU Machine Protocol (QMP) commands
------------------------------------
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index 96533886b1..50a5a40dbc 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -112,30 +112,61 @@ void machine_parse_smp_config(MachineState *ms,
/*
* If not supported by the machine, a topology parameter must be
- * omitted or specified equal to 1.
+ * omitted.
*/
- if (!mc->smp_props.dies_supported && dies > 1) {
- error_setg(errp, "dies not supported by this machine's CPU topology");
- return;
+ if (!mc->smp_props.clusters_supported && config->has_clusters) {
+ if (config->clusters > 1) {
+ error_setg(errp, "clusters not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here clusters only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported clusters parameter mustn't be "
+ "specified as 1");
+ }
}
- if (!mc->smp_props.clusters_supported && clusters > 1) {
- error_setg(errp, "clusters not supported by this machine's CPU topology");
- return;
- }
-
- dies = dies > 0 ? dies : 1;
clusters = clusters > 0 ? clusters : 1;
- if (!mc->smp_props.books_supported && books > 1) {
- error_setg(errp, "books not supported by this machine's CPU topology");
- return;
+ if (!mc->smp_props.dies_supported && config->has_dies) {
+ if (config->dies > 1) {
+ error_setg(errp, "dies not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here dies only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported dies parameter mustn't be "
+ "specified as 1");
+ }
+ }
+ dies = dies > 0 ? dies : 1;
+
+ if (!mc->smp_props.books_supported && config->has_books) {
+ if (config->books > 1) {
+ error_setg(errp, "books not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here books only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported books parameter mustn't be "
+ "specified as 1");
+ }
}
books = books > 0 ? books : 1;
- if (!mc->smp_props.drawers_supported && drawers > 1) {
- error_setg(errp,
- "drawers not supported by this machine's CPU topology");
- return;
+ if (!mc->smp_props.drawers_supported && config->has_drawers) {
+ if (config->drawers > 1) {
+ error_setg(errp, "drawers not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here drawers only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported drawers parameter mustn't be "
+ "specified as 1");
+ }
}
drawers = drawers > 0 ? drawers : 1;
--
2.41.0
8 months, 2 weeks
[PULL 30/43] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations
by Philippe Mathieu-Daudé
From: Zhao Liu <zhao1.liu(a)intel.com>
The "parameter=0" SMP configurations have been marked as deprecated
since v6.2.
For these cases, -smp currently returns the warning and adjusts the
zeroed parameters to 1 by default.
Remove the above compatibility logic in v9.0, and return error directly
if any -smp parameter is set as 0.
Signed-off-by: Zhao Liu <zhao1.liu(a)intel.com>
Reviewed-by: Thomas Huth <thuth(a)redhat.com>
Reviewed-by: Prasad Pandit <pjp(a)fedoraproject.org>
Message-ID: <20240308160148.3130837-2-zhao1.liu(a)linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
---
docs/about/deprecated.rst | 16 ----------------
docs/about/removed-features.rst | 15 +++++++++++++++
hw/core/machine-smp.c | 5 +++--
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 8565644da6..6e2f557682 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -36,22 +36,6 @@ and will cause a warning.
The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
rather than ``delay=off``.
-``-smp`` ("parameter=0" SMP configurations) (since 6.2)
-'''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-Specified CPU topology parameters must be greater than zero.
-
-In the SMP configuration, users should either provide a CPU topology
-parameter with a reasonable value (greater than zero) or just omit it
-and QEMU will compute the missing value.
-
-However, historically it was implicitly allowed for users to provide
-a parameter with zero value, which is meaningless and could also possibly
-cause unexpected results in the -smp parsing. So support for this kind of
-configurations (e.g. -smp 8,sockets=0) is deprecated since 6.2 and will
-be removed in the near future, users have to ensure that all the topology
-members described with -smp are greater than zero.
-
Plugin argument passing through ``arg=<string>`` (since 6.1)
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 417a0e4fa1..f9cf874f7b 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -489,6 +489,21 @@ The ``-singlestep`` option has been turned into an accelerator property,
and given a name that better reflects what it actually does.
Use ``-accel tcg,one-insn-per-tb=on`` instead.
+``-smp`` ("parameter=0" SMP configurations) (removed in 9.0)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Specified CPU topology parameters must be greater than zero.
+
+In the SMP configuration, users should either provide a CPU topology
+parameter with a reasonable value (greater than zero) or just omit it
+and QEMU will compute the missing value.
+
+However, historically it was implicitly allowed for users to provide
+a parameter with zero value, which is meaningless and could also possibly
+cause unexpected results in the -smp parsing. So support for this kind of
+configurations (e.g. -smp 8,sockets=0) is removed since 9.0, users have
+to ensure that all the topology members described with -smp are greater
+than zero.
User-mode emulator command line arguments
-----------------------------------------
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index 25019c91ee..96533886b1 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -105,8 +105,9 @@ void machine_parse_smp_config(MachineState *ms,
(config->has_cores && config->cores == 0) ||
(config->has_threads && config->threads == 0) ||
(config->has_maxcpus && config->maxcpus == 0)) {
- warn_report("Deprecated CPU topology (considered invalid): "
- "CPU topology parameters must be greater than zero");
+ error_setg(errp, "Invalid CPU topology: "
+ "CPU topology parameters must be greater than zero");
+ return;
}
/*
--
2.41.0
8 months, 2 weeks
RE: join running core dump job
by Thanos Makatos
> -----Original Message-----
> From: Thanos Makatos <thanos.makatos(a)nutanix.com>
> Sent: Monday, March 4, 2024 9:45 PM
> To: Thanos Makatos <thanos.makatos(a)nutanix.com>; devel(a)lists.libvirt.org
> Subject: RE: join running core dump job
>
> > -----Original Message-----
> > From: Thanos Makatos <thanos.makatos(a)nutanix.com>
> > Sent: Monday, March 4, 2024 5:24 PM
> > To: devel(a)lists.libvirt.org
> > Subject: join running core dump job
> >
> > Is there a way to programmatically wait for a previously initiated
> > virDomainCoreDumpWithFormat() where the process that started it died?
> I'm
> > looking at the API and don't seem to find anything relevant. I suppose I
> could
> > poll via virDomainGetJobStats(), but, ideally, I'd like a function that would
> join
> > the dump job and return when the dump job finishes.
> > _______________________________________________
> > Devel mailing list -- devel(a)lists.libvirt.org
> > To unsubscribe send an email to devel-leave(a)lists.libvirt.org
>
> I see there's qemuDumpWaitForCompletion(), looks promising.
I've made some progress (added a virHypervisorDriver.domainCoreDumpWait and the relevant scaffolding to make 'virsh dump --wait' work), calling qemuDumpWaitForCompletion() is all that's needed.
However, it doesn't seem trivial to implement this in the test_driver.
First, IIUC testDomainCoreDumpWithFormat() gets an exclusive lock on the domain (haven't tested anything yet), so calling domainCoreDumpWait() would block for the wrong reason. Is making testDomainCoreDumpWithFormat() using asynchronous jobs internally unavoidable?
Second, I want to test behaviour in my application where (1) it calls domainCoreDump(), (2) crashes before domainCoreDump() finishes, (3) then my application starts again and looks for a pending dump job and (4) joins it using domainCoreDumpWait(). I can't see an easy wait of faking a dump job in the test_driver when it starts. How about adding persistent tasks, which I can pre-populate before starting my application, or fake jobs via an environment variable, so that when the test_driver starts it can internally continue them? E.g. we can specify how long to run the job for and domainCoreDumpWait() add a sleep for that long.
I'm open to suggestions.
8 months, 2 weeks
[PATCH 0/2] ch_driver: add basic save and restore functionalities
by Purna Pavan Chandra Aekkaladevi
Purna Pavan Chandra Aekkaladevi (2):
ch_driver: Add basic domain save & restore support
NEWS: Mention save & restore support for ch driver
NEWS.rst | 6 +
src/ch/ch_conf.c | 6 +
src/ch/ch_conf.h | 12 ++
src/ch/ch_driver.c | 511 +++++++++++++++++++++++++++++++++++++++++++-
src/ch/ch_monitor.c | 97 ++++++++-
src/ch/ch_monitor.h | 6 +-
src/ch/ch_process.c | 101 +++++++--
src/ch/ch_process.h | 4 +
8 files changed, 721 insertions(+), 22 deletions(-)
--
2.34.1
8 months, 2 weeks
[libvirt PATCH 0/3] acouple of net-metadata fixes
by Ján Tomko
Ján Tomko (3):
remote: add VIR_ERR_NO_NETWORK_METADATA to daemonErrorLogFilter
virsh: remove trailing whitespace even when editing the description
vsh: introduce vshEditString
src/remote/remote_daemon.c | 1 +
tools/virsh-domain.c | 21 +--------------------
tools/virsh-network.c | 20 +-------------------
tools/vsh.c | 27 +++++++++++++++++++++++++++
tools/vsh.h | 1 +
5 files changed, 31 insertions(+), 39 deletions(-)
--
2.43.2
8 months, 2 weeks
[libvirt PATCH V3 0/4] add loongarch support for libvirt
by Xianglai Li
Hello, Everyone:
This patch series adds libvirt support for loongarch.Although the bios
path and name has not been officially integrated into qemu and we think
there are still many shortcomings, we try to push a version of patch to
the community according to the opinions of the community, hoping to
listen to everyone's opinions. Anyway we have a version of libvirt that
supports loongarch.
You can also get libvirt's patch from the link below:
https://gitlab.com/lixianglai/libvirt
branch: loongarch
Since the patch associated with loongarch has not yet been submitted to
the virt-manager community, we are providing a temporary patch with
loongarch for the time being patch's virt-manager, the open source work
of virt-manager adding loongarch will be followed up later or
synchronized with the open source libvirt.
You can get the virt-manager code with loongarch patch from the link below:
https://github.com/lixianglai/virt-manager
branch: loongarch
loongarch's virtual machine bios is not yet available in qemu,So you need to compile loongarch UEFI yourself,
you can refer to the following link to compile UEFI:
https://github.com/tianocore/edk2-platforms/blob/master/Platform/Loongson...
Here we provide compiled UEFI for ease of testing,you can get it from the following link:
https://github.com/lixianglai/LoongarchVirtFirmware
(Note: You should clone the repository using git instead of downloading the file via wget or you'll get xml)
We named the bios QEMU_EFI.fd, QEMU_VARS.fd is used to store pflash images of non-volatile variables.
After installing qemu-system-loongarch64,You can install the loongarch bios by executing the script
install-loongarch-virt-firmware.sh
Since there is no fedora operating system that supports the loongarch
architecture, you can find an iso that supports loongarch at the link below for testing purposes:
https://github.com/fedora-remix-loongarch/releases-info
The operating system provided above may have minor problems of one kind or another,
but you can also find out from the following link that openEuler's operating system supports loongarch:
https://muug.ca/mirror/openeuler/openEuler-22.03-LTS/ISO/loongarch64/
Well, if you have completed the above steps I think you can now install loongarch virtual machine,
you can install it through the virt-manager graphical interface, or install it through vrit-install,
here is an example of installing it using virt-install:
virt-install \
--virt-type=qemu \
--name loongarch-test \
--memory 4096 \
--vcpus=4 \
--arch=loongarch64 \
--boot cdrom \
--disk device=cdrom,bus=scsi,path=/root/livecd-fedora-mate-4.loongarch64.iso \
--disk path=/var/lib/libvirt/images/debian12-loongarch64.qcow2,size=10,format=qcow2,bus=scsi \
--network network=default \
--osinfo archlinux \
--video=virtio \
--graphics=vnc,listen=0.0.0.0
CHANGES
V2->V3:
1.Delete redundant header files in cpu_loongarch.c
2.Fixed code formatting issues
3.Modify the commit message of [PATCH 2/4]
4.Remove useless test cases for loongarch,
Rebuild loongarch's test case based on the latest code.
V1->V2:
1.Modify the link addresses of virtu-manager and firmeware in the cover
letter. Please obtain the code and firmware from the latest link
address.
2.Rename the bios name. Delete the loongarch bios name from libvirt
and use the json file to obtain the bios path.
3.Refer to riscv64 to simplify the implementation of loongarch cpu
driver.And fix some code style errors.
4.Delete unnecessary or redundant device enablement.Such as USB NEC.
5.Add some test cases for loongarch.
Xianglai Li (4):
Add loongarch cpu support
Support for loongarch64 in the QEMU driver
Implement the method of getting host info for loongarch
Add test script for loongarch
src/conf/schemas/basictypes.rng | 1 +
src/cpu/cpu.c | 2 +
src/cpu/cpu_loongarch.c | 58 +
src/cpu/cpu_loongarch.h | 25 +
src/cpu/meson.build | 1 +
src/qemu/qemu_capabilities.c | 16 +-
src/qemu/qemu_command.c | 7 +-
src/qemu/qemu_domain.c | 40 +-
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_validate.c | 1 +
src/util/virarch.c | 13 +-
src/util/virarch.h | 13 +-
src/util/virhostcpu.c | 7 +-
src/util/virsysinfo.c | 3 +-
.../qemu_8.2.0-tcg-virt.loongarch64.xml | 163 +
.../qemu_8.2.0-virt.loongarch64.xml | 167 +
tests/domaincapstest.c | 4 +-
.../caps_8.2.0_loongarch64.replies | 30121 ++++++++++++++++
.../caps_8.2.0_loongarch64.xml | 175 +
.../qemucaps2xmloutdata/caps.loongarch64.xml | 28 +
...o-type-loongarch64.loongarch64-latest.args | 34 +
...eo-type-loongarch64.loongarch64-latest.xml | 45 +
.../default-video-type-loongarch64.xml | 18 +
...-models.loongarch64-latest.abi-update.args | 44 +
...t-models.loongarch64-latest.abi-update.xml | 79 +
...irt-default-models.loongarch64-latest.args | 44 +
...virt-default-models.loongarch64-latest.xml | 79 +
.../loongarch64-virt-default-models.xml | 24 +
...ch64-virt-graphics.loongarch64-latest.args | 56 +
...rch64-virt-graphics.loongarch64-latest.xml | 116 +
.../loongarch64-virt-graphics.xml | 48 +
...ch64-virt-headless.loongarch64-latest.args | 52 +
...rch64-virt-headless.loongarch64-latest.xml | 102 +
.../loongarch64-virt-headless.xml | 42 +
...minimal.loongarch64-latest.abi-update.args | 31 +
...-minimal.loongarch64-latest.abi-update.xml | 26 +
...rch64-virt-minimal.loongarch64-latest.args | 31 +
...arch64-virt-minimal.loongarch64-latest.xml | 26 +
.../loongarch64-virt-minimal.xml | 15 +
tests/qemuxmlconftest.c | 7 +
tests/testutilshostcpus.h | 10 +
41 files changed, 31749 insertions(+), 26 deletions(-)
create mode 100644 src/cpu/cpu_loongarch.c
create mode 100644 src/cpu/cpu_loongarch.h
create mode 100644 tests/domaincapsdata/qemu_8.2.0-tcg-virt.loongarch64.xml
create mode 100644 tests/domaincapsdata/qemu_8.2.0-virt.loongarch64.xml
create mode 100644 tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.replies
create mode 100644 tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml
create mode 100644 tests/qemucaps2xmloutdata/caps.loongarch64.xml
create mode 100644 tests/qemuxmlconfdata/default-video-type-loongarch64.loongarch64-latest.args
create mode 100644 tests/qemuxmlconfdata/default-video-type-loongarch64.loongarch64-latest.xml
create mode 100644 tests/qemuxmlconfdata/default-video-type-loongarch64.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-default-models.loongarch64-latest.abi-update.args
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-default-models.loongarch64-latest.abi-update.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-default-models.loongarch64-latest.args
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-default-models.loongarch64-latest.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-default-models.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-graphics.loongarch64-latest.args
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-graphics.loongarch64-latest.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-graphics.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-headless.loongarch64-latest.args
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-headless.loongarch64-latest.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-headless.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-minimal.loongarch64-latest.abi-update.args
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-minimal.loongarch64-latest.abi-update.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-minimal.loongarch64-latest.args
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-minimal.loongarch64-latest.xml
create mode 100644 tests/qemuxmlconfdata/loongarch64-virt-minimal.xml
--
2.39.1
8 months, 2 weeks
