[libvirt PATCH] qemuProcessUpdateGuestCPU: Check cpu if check=full is set
libvirt performs cpu checking if "check" is set to "partial", but skips
checking the cpu if "check" is set to "full".
See https://bugzilla.redhat.com/show_bug.cgi?id=1840770
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_process.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bfa742577f..5b8c1397ef 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6149,6 +6149,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
if (virCPUConvertLegacy(hostarch, def->cpu) < 0)
return -1;
+if (def->cpu->check == VIR_CPU_CHECK_FULL) {
+virCPUDefPtr host = virQEMUCapsGetHostModel(qemuCaps, def->virtType,
+
VIR_QEMU_CAPS_HOST_CPU_FULL);
+
+if (virCPUCompare(hostarch, host, def->cpu, true) < 0)
+return -1;
+}
+
/* nothing to update for host-passthrough / maximum */
if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
def->cpu->mode != VIR_CPU_MODE_MAXIMUM) {
--
2.26.2
Re: [PATCH v2] qemu: Add missing lock in qemuProcessHandleMonitorEOF
On 2/25/21 2:39 AM, Peng Liang wrote: On 2/24/2021 9:59 PM, Michal Privoznik wrote: On 2/24/21 12:28 PM, Peng Liang wrote: qemuMonitorUnregister will be called in multiple threads (e.g. threads in rpc worker pool and the vm event thread). In some cases, it isn't protected by the monitor lock, which may lead to call g_source_unref more than one time and a use-after-free problem eventually. Add the missing lock in qemuProcessHandleMonitorEOF (which is the only position missing lock of monitor I found). Suggested-by: Michal Privoznik Signed-off-by: Peng Liang --- This patch is v2 of https://listman.redhat.com/archives/libvir-list/2021-February/msg00945.html. v1 -> v2: Locking monitor in qemuProcessHandleMonitorEOF instead of using aotmic function in qemuMonitorUnregister. src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_process.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d25c26343a7f..14e6b1fe9626 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -427,6 +427,7 @@ bool qemuMonitorWasDisposed(void); void qemuMonitorRegister(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); +/* Must be called with monitor locked. */ void qemuMonitorUnregister(qemuMonitorPtr mon) From this it's not very clear which function the comment belongs to. We tend to put comments into .c because that's where tags usually take you first. So you get the memo first. Hi Michal, So, do I need to send another patch to document it in src/qemu/qemu_monitor.c? No need I just did: https://listman.redhat.com/archives/libvir-list/2021-February/msg01202.html I'll merge it later today. Michal
Re: [PATCH 04/14] softmmu: remove '-usbdevice' command line option
On 24/02/2021 15.10, Daniel P. Berrangé wrote: On Wed, Feb 24, 2021 at 02:58:19PM +0100, Thomas Huth wrote: On 24/02/2021 14.11, Daniel P. Berrangé wrote: This was replaced by the '-device usb-DEV' option. Signed-off-by: Daniel P. Berrangé --- docs/system/deprecated.rst | 9 --- docs/system/removed-features.rst | 9 +++ softmmu/vl.c | 42 3 files changed, 9 insertions(+), 51 deletions(-) Last time I tried to remove -usbdevice, there was some concerns that -usbdevice braille might still be useful for some people, see the thread that started here: https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00651.html (and Gerd's summary here: https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg01520.html ) Urgh, so the current deprecation docs are a bit misleading by saying -usbdevice is directly mapped to -device. So we might need a new "sugared" option like "-braille" instead before we can fully remove -usbdevice? ... or we just keep -usbdevice as a bittersweet remainder? I'm not going to implement new CLI options, and if that's needed, we ought to re-start the clock on the deprecation at that point. So this points towards just removing the deprecation warning that exists today. Or alternatively drop support for -usbdevice, except for the braille type. After that discussion in 2018, I've removed all of the "annoying" -usbdevice parameters already (see commit 99761176eeaf8525). I then more or less waited for someone to step up and implement "-braille", but it never happened and I forgot about the removal of the remaining -usbdevice parameters. Thinking about this again, replacing "-usbdevice braille" with a "-braille usb" does indeed not buy us much, so I think the best is maybe to keep the simple devices and braille around, update our documentation and remove the deprecation warning instead. Thomas
Re: [PATCH 33/33] util: virerror: Remove virReportOOMError
On 2/24/21 11:17 AM, Peter Krempa wrote:
Trying to report an OOM error is pointless since our infrastructure to
report error needs to allocate memory to report the error.
In addition our code mistakenly reported OOM errors even in cases where
a function could fail for another reason, which would make issues harder
to debug.
Remove the virReportOOMError and backend so that programmers are forced
to think about what can happen. In case when there's another failure
possible a specific error should be reported and otherwise a direct
abort() is better since the logger would abort on g_new anyways.
This patch also removes the syntas-check which forces use of
virReportOOMError instead of using VIR_ERR_NO_MEMORY with other
functions. This allows possible future use when we'd end up in a
situation where trying to recover from an OOM would make sense, such as
when attempting to allocate a massive buffer.
Signed-off-by: Peter Krempa
and good riddance!
Reviewed-by: Laine Stump
---
build-aux/syntax-check.mk | 8
src/libvirt_private.syms | 1 -
src/util/virerror.c | 22 --
src/util/virerror.h | 8
4 files changed, 39 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index e51877648a..e1ccb74986 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -490,11 +490,6 @@ sc_prohibit_gettext_noop:
halt='use N_, not gettext_noop' \
$(_sc_search_regexp)
-sc_prohibit_VIR_ERR_NO_MEMORY:
- @prohibit='\' \
- halt='use virReportOOMError, not VIR_ERR_NO_MEMORY' \
- $(_sc_search_regexp)
-
sc_prohibit_PATH_MAX:
@prohibit='\' \
halt='dynamically allocate paths, do not use PATH_MAX' \
@@ -1895,9 +1890,6 @@ exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics
= \
exclude_file_name_regexp--sc_po_check =
^(docs/|src/rpc/gendispatch\.pl$$|tests/commandtest.c$$)
-exclude_file_name_regexp--sc_prohibit_VIR_ERR_NO_MEMORY = \
-
^(build-aux/syntax-check\.mk|include/libvirt/virterror\.h|src/remote/remote_daemon_dispatch\.c|src/util/virerror\.c|docs/internals/oomtesting\.html\.in)$$
-
exclude_file_name_regexp--sc_prohibit_PATH_MAX = \
^build-aux/syntax-check\.mk$$
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 48f66daab8..7eb37ed797 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2116,7 +2116,6 @@ virLastErrorPrefixMessage;
virRaiseErrorFull;
virRaiseErrorObject;
virReportErrorHelper;
-virReportOOMErrorFull;
virReportSystemErrorFull;
virSetError;
virSetErrorLogPriorityFunc;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 708081414a..a503cdefdc 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1358,28 +1358,6 @@ void virReportSystemErrorFull(int domcode,
errno = save_errno;
}
-/**
- * virReportOOMErrorFull:
- * @domcode: the virErrorDomain indicating where it's coming from
- * @filename: filename where error was raised
- * @funcname: function name where error was raised
- * @linenr: line number where error was raised
- *
- * Convenience internal routine called when an out of memory error is
- * detected
- */
-void virReportOOMErrorFull(int domcode,
- const char *filename,
- const char *funcname,
- size_t linenr)
-{
-const char *virerr;
-
-virerr = virErrorMsg(VIR_ERR_NO_MEMORY, NULL);
-virRaiseErrorFull(filename, funcname, linenr,
- domcode, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
- virerr, NULL, NULL, -1, -1, virerr, NULL);
-}
/**
* virSetErrorLogPriorityFunc:
diff --git a/src/util/virerror.h b/src/util/virerror.h
index 9d3e40d65a..da7d7c0afe 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -174,14 +174,6 @@ void virReportSystemErrorFull(int domcode,
"Unexpected enum value %d for %s", \
value, sizeof((typname)1) != 0 ? #typname :
#typname);
-void virReportOOMErrorFull(int domcode,
- const char *filename,
- const char *funcname,
- size_t linenr);
-
-#define virReportOOMError() \
-virReportOOMErrorFull(VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__)
-
#define virReportError(code, ...) \
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
Re: [PATCH 32/33] virVMXConvertToUTF8: Report non-OOM error on failure of xmlBufferCreateStatic
On 2/24/21 11:17 AM, Peter Krempa wrote:
The function has also non-OOM failure case when the passed string has 0
length, so reporting OOM error is not correct.
Signed-off-by: Peter Krempa
---
src/vmx/vmx.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index e6c0900a65..73bf7c4f3d 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -781,12 +781,8 @@ virVMXConvertToUTF8(const char *encoding, const char
*string)
return NULL;
}
-if (!(input = xmlBufferCreateStatic((char *)string, strlen(string {
-virReportOOMError();
-goto cleanup;
-}
-
-if (xmlCharEncInFunc(handler, utf8, input) < 0) {
+if (!(input = xmlBufferCreateStatic((char *)string, strlen(string))) ||
+xmlCharEncInFunc(handler, utf8, input) < 0) {
I would have left the error messages for the two functions separate,
with adequate wording to tell which of them failed. But this is at worst
still better than reporting OOM...
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not convert from %s to UTF-8 encoding"),
encoding);
goto cleanup;
Re: [PATCH 20/33] Don't report OOM error on xmlCopyNode failure
On 2/24/21 11:16 AM, Peter Krempa wrote:
Out of memory isn't the only reason the function can fail. Add a message
stating that copying of a XML node failed.
Could it still fail due to OOM as well, though? And if so, is there any
way of differentiating? (I previously looked into this for some other
libxml2 function, ended up asking DV, and I believe I was told that
there isn't any way to tell the difference, but as usual IMBES (I May Be
Experiencing Senility).
At any rate, your changed message is more correct, since it's not making
any unsubstantiated assumption about the cause of the error, while
virRemoveOOMError() was).
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 3 ++-
src/test/test_driver.c | 6 --
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c0881608af..f1fd5320a5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30460,7 +30460,8 @@ virDomainDefSetMetadata(virDomainDefPtr def,
def->metadata = virXMLNewNode(NULL, "metadata");
if (!(new = xmlCopyNode(doc->children, 1))) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to copy XML node"));
return -1;
}
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index bca1297d1d..71ab04aa1a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -224,7 +224,8 @@ testDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
for (i = 0; i < n; i++) {
xmlNodePtr newnode = xmlCopyNode(nodes[i], 1);
if (!newnode) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to copy XML node"));
goto error;
}
@@ -787,7 +788,8 @@ testParseXMLDocFromFile(xmlNodePtr node, const char *file,
const char *type)
ret = xmlCopyNode(xmlDocGetRootElement(doc), 1);
if (!ret) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to copy XML node"));
goto error;
}
xmlReplaceNode(node, ret);
Re: [PATCH v2] qemu: Add missing lock in qemuProcessHandleMonitorEOF
On 2/24/2021 9:59 PM, Michal Privoznik wrote: > On 2/24/21 12:28 PM, Peng Liang wrote: >> qemuMonitorUnregister will be called in multiple threads (e.g. threads >> in rpc worker pool and the vm event thread). In some cases, it isn't >> protected by the monitor lock, which may lead to call g_source_unref >> more than one time and a use-after-free problem eventually. >> >> Add the missing lock in qemuProcessHandleMonitorEOF (which is the only >> position missing lock of monitor I found). >> >> Suggested-by: Michal Privoznik >> Signed-off-by: Peng Liang >> --- >> This patch is v2 of >> https://listman.redhat.com/archives/libvir-list/2021-February/msg00945.html. >> >> >> v1 -> v2: >> Locking monitor in qemuProcessHandleMonitorEOF instead of using aotmic >> function in qemuMonitorUnregister. >> >> src/qemu/qemu_monitor.h | 1 + >> src/qemu/qemu_process.c | 2 ++ >> 2 files changed, 3 insertions(+) >> >> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h >> index d25c26343a7f..14e6b1fe9626 100644 >> --- a/src/qemu/qemu_monitor.h >> +++ b/src/qemu/qemu_monitor.h >> @@ -427,6 +427,7 @@ bool qemuMonitorWasDisposed(void); >> void qemuMonitorRegister(qemuMonitorPtr mon) >> ATTRIBUTE_NONNULL(1); >> +/* Must be called with monitor locked. */ >> void qemuMonitorUnregister(qemuMonitorPtr mon) > > From this it's not very clear which function the comment belongs to. We > tend to put comments into .c because that's where tags usually take you > first. So you get the memo first. Hi Michal, So, do I need to send another patch to document it in src/qemu/qemu_monitor.c? Thanks, Peng > >> ATTRIBUTE_NONNULL(1); >> void qemuMonitorClose(qemuMonitorPtr mon); >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index d930ff9a74f6..bfa742577f32 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -318,7 +318,9 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon, >> /* We don't want this EOF handler to be called over and over >> while the >> * thread is waiting for a job. >> */ >> + virObjectLock(mon); >> qemuMonitorUnregister(mon); >> + virObjectUnlock(mon); >> /* We don't want any cleanup from EOF handler (or any other >> * thread) to enter qemu namespace. */ >> > > ACK to this hunk. And I'll be pushing it in a few moments. > > Michal > > .
Re: [PATCH 00/33] Remove the now misleading virReportOOMError infra
On 2/24/21 11:16 AM, Peter Krempa wrote: 'virReportOOMError' is nowadays mostly useless since an OOM error will trigger yet another allocation failure in the logger when attempting to log the error. Additionally it was also used in few places which can fail with other failures than OOM. To prevent both errorneous usage types, let's remove it completely. Yay!! It bugs me every time I see it.
Re: [PATCH 3/3] Remove redundant variables/labels
On 2/23/21 12:24 PM, Kristina Hanicova wrote: In files: src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(), src/conf/networkcommon_conf: in virNetDevIPRouteCreate() and virNetDevIPRouteParseXML() Signed-off-by: Kristina Hanicova Reviewed-by: Laine Stump
Re: [PATCH 2/3] Use g_autoptr instead of virNetDevIPRouteFree if possible
On 2/23/21 12:24 PM, Kristina Hanicova wrote:
In files: src/conf/domain_conf: in virDomainNetIPInfoParseXML(),
src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(),
src/vz/vz_sdk: in prlsdkGetRoutes(), src/conf/networkcommon_conf:
in virNetDevIPRouteCreate()
Signed-off-by: Kristina Hanicova
---
src/conf/domain_conf.c| 3 +--
src/conf/networkcommon_conf.c | 5 ++---
src/lxc/lxc_native.c | 3 +--
src/vz/vz_sdk.c | 3 +--
4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b731744f04..2504911931 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7481,7 +7481,7 @@ virDomainNetIPInfoParseXML(const char *source,
xmlXPathContextPtr ctxt,
virNetDevIPInfoPtr def)
Not your problem, but I noticed that this function previously had the
local "nodes" converted to g_autofree, apparently before the rule about
"don't VIR_FREE an g_auto pointer" (because the change was made by the
person who later called me on not following the rule :-))
Anyway, it wouldn't fit the theme to fix that in this patch, so it can
be cleaned up later.
{
-virNetDevIPRoutePtr route = NULL;
+g_autoptr(virNetDevIPRoute) route = NULL;
int nnodes;
int ret = -1;
size_t i;
@@ -7511,7 +7511,6 @@ virDomainNetIPInfoParseXML(const char *source,
cleanup:
if (ret < 0)
virNetDevIPInfoClear(def);
-virNetDevIPRouteFree(route);
return ret;
}
diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c
index e82dbc3d3d..9e7ad59337 100644
--- a/src/conf/networkcommon_conf.c
+++ b/src/conf/networkcommon_conf.c
@@ -41,7 +41,7 @@ virNetDevIPRouteCreate(const char *errorDetail,
unsigned int metric,
bool hasMetric)
{
-virNetDevIPRoutePtr def = NULL;
+g_autoptr(virNetDevIPRoute) def = NULL;
virSocketAddr testAddr;
def = g_new0(virNetDevIPRoute, 1);
@@ -209,10 +209,9 @@ virNetDevIPRouteCreate(const char *errorDetail,
goto error;
}
-return def;
+return g_steal_pointer();
error:
-virNetDevIPRouteFree(def);
return NULL;
}
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index b903dc91d6..d5020dafa7 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -441,7 +441,7 @@ lxcAddNetworkRouteDefinition(const char *address,
virNetDevIPRoutePtr **routes,
size_t *nroutes)
{
-virNetDevIPRoutePtr route = NULL;
+g_autoptr(virNetDevIPRoute) route = NULL;
g_autofree char *familyStr = NULL;
g_autofree char *zero = NULL;
@@ -460,7 +460,6 @@ lxcAddNetworkRouteDefinition(const char *address,
return 0;
error:
-virNetDevIPRouteFree(route);
return -1;
}
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f49cd983f0..6f712c7a31 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -974,7 +974,7 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
int ret = -1;
char *gw = NULL;
char *gw6 = NULL;
-virNetDevIPRoutePtr route = NULL;
+g_autoptr(virNetDevIPRoute) route = NULL;
Interesting. So this function uses route multiple times (once for IPv4
and once for IPv6), but it's clearing out the first usage by stealing
the pointer into and array of routes rather than freeing it. I guess
that's okay, though.
Anyway:
Reviewed-by: Laine Stump
I'll put this in a branch of things to push after the freeze is over.
if (!(gw = prlsdkGetStringParamVar(PrlVmDevNet_GetDefaultGateway, sdknet)))
goto cleanup;
@@ -1006,7 +1006,6 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
ret = 0;
cleanup:
-virNetDevIPRouteFree(route);
VIR_FREE(gw);
VIR_FREE(gw6);
Re: [PATCH 1/3] networkcommon_conf: Use g_autofree where possible
On 2/23/21 12:24 PM, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova
---
src/conf/networkcommon_conf.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c
index 26eeb6dbda..e82dbc3d3d 100644
--- a/src/conf/networkcommon_conf.c
+++ b/src/conf/networkcommon_conf.c
@@ -228,9 +228,10 @@ virNetDevIPRouteParseXML(const char *errorDetail,
virNetDevIPRoutePtr def = NULL;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-char *family = NULL;
-char *address = NULL, *netmask = NULL;
-char *gateway = NULL;
+g_autofree char *family = NULL;
+g_autofree char *address = NULL;
+g_autofree char *netmask = NULL;
+g_autofree char *gateway = NULL;
unsigned long prefix = 0, metric = 0;
int prefixRc, metricRc;
bool hasPrefix = false;
@@ -276,10 +277,6 @@ virNetDevIPRouteParseXML(const char *errorDetail,
hasMetric);
cleanup:
-VIR_FREE(family);
-VIR_FREE(address);
-VIR_FREE(netmask);
-VIR_FREE(gateway);
return def;
}
@@ -287,7 +284,7 @@ int
virNetDevIPRouteFormat(virBufferPtr buf,
const virNetDevIPRoute *def)
{
-char *addr = NULL;
+g_autofree char *addr = NULL;
virBufferAddLit(buf, "
@@ -311,7 +308,6 @@ virNetDevIPRouteFormat(virBufferPtr buf,
if (!(addr = virSocketAddrFormat(>gateway)))
return -1;
virBufferAsprintf(buf, " gateway='%s'", addr);
-VIR_FREE(addr);
This one is a bit more complicated, because "addr" is re-used after
being freed (twice - first used for address, then for netmask, and
finally for gateway), and we've made the rule that an auto-freed pointer
should never be VIR_FREED
So instead of just making the existing pointer g_autofree and
eliminating its final free, we need to create separate pointers for each
use:
g_autofree char *address = NULL;
g_autofree char *netmask = NULL;
g_autofree char *gataeway = NULL;
and then use each in the appropriate place.
(if we're lucky, the compiler/optimizer will even be smart enough to
figure out that the three things are never used at the same time (?
depending on when the call to the autofree function is injected), and
internally merge them into a single variable.)
if (def->has_metric && def->metric > 0)
virBufferAsprintf(buf, " metric='%u'", def->metric);
Re: [libvirt PATCH v4 00/25] Add support for persistent mediated devices
On Mon, 22 Feb 2021 11:08:12 +0100 Erik Skultety wrote: > On Wed, Feb 03, 2021 at 11:38:44AM -0600, Jonathon Jongsma wrote: > > This patch series follows the previously-merged series which added > > support for transient mediated devices. This series expands mdev > > support to include persistent device definitions. Again, it relies > > on mdevctl as the backend. > > > > It follows the common libvirt pattern of APIs by adding the > > following new APIs for node devices: > > - virNodeDeviceDefineXML() - defines a persistent device > > - virNodeDeviceUndefine() - undefines a persistent device > > - virNodeDeviceCreate() - starts a previously-defined device > > So, I tested the patches on Friday and have a few notes: > - all of the driver implementations mentioned ^above need to pass an > error buffer to the respective mdevctl wrapper, because the generic > error "Unable to XZY mediated device" doesn't help at all. Kind of > like with QEMU errors, we need to extract it from an error buffer > (you already have input and output, so just add another one) --> this > relates to patches 14,18,21 > > - trying to undefine an active mdev reports an error in libvirt > --> this is both inconsistent with the rest of libvirt's > subsystems and there's also no reason for it since mdevctl supports it > --> we need to support transient devices as well > > - trying to define the following XML hangs for 5s and then fails: > > pci__06_00_0 > > vfio_mdev > > > > d41787d2-2e0e-4021-8ed2-b6f1ef305a9f > > I assume that you have a parent device that supports creating an mdev of this type? In other words, this was expected to succeed, right? > > -> I debugged this on Friday evening and for some reason > driver->devs hash table of devices was NULL going through the > nodeDeviceFindNewMediatedDevice call, but I haven't managed to figure > out why it was NULL, listing devices worked just fine So, I often get a 5s delay when trying to define a new mdev. But it always succeeds after the delay. The reason for the delay is that the device is generally not in our device list the first time we call nodeDeviceFindNewMediatedDevice(). It's usually not in our device list because the device is only added to the driver's device list after we get a notification from mdevctl (via monitoring the mdevctl config directory) and then we re-query mdevctl for the list of defined devices. Because mdevctl is not blazingly fast[1], and because we do the querying in a separate thread, the new device has usually not been discovered when we first call nodeDeviceFindNewMediatedDevice(). So we sleep for 5s before trying again. For udev devices, this 5s delay is usually not hit because nodeDeviceFindNewDevice() first calls 'udevadm settle' before checking the device list. This waits just long enough to ensure that pending udev events are handled. Unfortunately we don't have a similar "wait just long enough" command for mdevctl, so we almost always hit the 5s retry timeout. There are a couple of potential ways to avoid hitting this 5s timeout: 1. directly add a placeholder device to the driver's device list immediately after calling 'mdevctl define' so that it is guaranteed to be in the device list when we call nodeDeviceFindNewMediatedDevice(). We can then update that device definition with additional info when mdevctl is eventually queried. 2. don't wait 5s every time a device is not found. Instead, start with a small sleep timeout and increase it gradually until we hit the max timeout. In other words, instead of check; sleep(5); check; sleep(5); check; sleep(5); we could instead do something like: check; sleep(1); check; sleep(2); check; sleep(4); check; sleep(8)... [1]$ time mdevctl list --defined 49bf2346-6747-4ad6-be5a-adc2f0a10b5c :00:02.0 i915-GVTg_V5_8 manual 4fcd3666-e58a-4c63-969c-bd616a158c0d :00:02.0 i915-GVTg_V5_8 manual 5c152919-3a34-4338-b7c9-532f73fa5440 :00:02.0 i915-GVTg_V5_4 manual real0m0.782s user0m0.735s sys 0m0.056s > > - I also prepared several adjustments to how we define the mdevctl > wrappers + some test adjustments which I wanted to share with you, > but I haven't tested those thoroughly since I was debugging why that > XML couldn't be defined, I'll share it when we eliminate the > underlying problem -> if you're wondering why I didn't just put it > into the review, well, I figured sharing the code was more > descriptive than if I used words Would you like me to wait for this before sending a new series? Thanks, Jonathon
[libvirt PATCH] qemu_domainjob: Make copy of owner API
Using the job owner API name directly works fine as long as it is a
static string or the owner's thread is still running. However, this is
not always the case. For example, when the owner API name is filled in a
job when we're reconnecting to existing domains after daemon restart,
the dynamically allocated owner name will disappear with the
reconnecting thread. Any follow up usage of the pointer will read random
memory.
Signed-off-by: Jiri Denemark
---
src/qemu/qemu_domainjob.c | 12 ++--
src/qemu/qemu_process.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
index 3c2c6b9179..b58d6837ad 100644
--- a/src/qemu/qemu_domainjob.c
+++ b/src/qemu/qemu_domainjob.c
@@ -190,7 +190,7 @@ qemuDomainObjResetJob(qemuDomainJobObjPtr job)
{
job->active = QEMU_JOB_NONE;
job->owner = 0;
-job->ownerAPI = NULL;
+g_clear_pointer(>ownerAPI, g_free);
job->started = 0;
}
@@ -200,7 +200,7 @@ qemuDomainObjResetAgentJob(qemuDomainJobObjPtr job)
{
job->agentActive = QEMU_AGENT_JOB_NONE;
job->agentOwner = 0;
-job->agentOwnerAPI = NULL;
+g_clear_pointer(>agentOwnerAPI, g_free);
job->agentStarted = 0;
}
@@ -210,7 +210,7 @@ qemuDomainObjResetAsyncJob(qemuDomainJobObjPtr job)
{
job->asyncJob = QEMU_ASYNC_JOB_NONE;
job->asyncOwner = 0;
-job->asyncOwnerAPI = NULL;
+g_clear_pointer(>asyncOwnerAPI, g_free);
job->asyncStarted = 0;
job->phase = 0;
job->mask = QEMU_JOB_DEFAULT_MASK;
@@ -890,7 +890,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
obj, obj->def->name);
priv->job.active = job;
priv->job.owner = virThreadSelfID();
-priv->job.ownerAPI = virThreadJobGet();
+priv->job.ownerAPI = g_strdup(virThreadJobGet());
priv->job.started = now;
} else {
VIR_DEBUG("Started async job: %s (vm=%p name=%s)",
@@ -901,7 +901,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
priv->job.asyncJob = asyncJob;
priv->job.asyncOwner = virThreadSelfID();
-priv->job.asyncOwnerAPI = virThreadJobGet();
+priv->job.asyncOwnerAPI = g_strdup(virThreadJobGet());
priv->job.asyncStarted = now;
priv->job.current->started = now;
}
@@ -917,7 +917,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
qemuDomainAsyncJobTypeToString(priv->job.asyncJob));
priv->job.agentActive = agentJob;
priv->job.agentOwner = virThreadSelfID();
-priv->job.agentOwnerAPI = virThreadJobGet();
+priv->job.agentOwnerAPI = g_strdup(virThreadJobGet());
priv->job.agentStarted = now;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bfa742577f..398f63282e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3732,7 +3732,7 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver,
/* Restore the config of the async job which is not persisted */
priv->jobs_queued++;
priv->job.asyncJob = QEMU_ASYNC_JOB_BACKUP;
-priv->job.asyncOwnerAPI = virThreadJobGet();
+priv->job.asyncOwnerAPI = g_strdup(virThreadJobGet());
priv->job.asyncStarted = now;
qemuDomainObjSetAsyncJobMask(vm, (QEMU_JOB_DEFAULT_MASK |
--
2.30.0
Re: [PATCH] qemu_monitor: Document qemuMonitorUnregister()
On Wed, 2021-02-24 at 15:04 +0100, Michal Privoznik wrote: > The most important bit is that the caller is expected to pass > locked monitor. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_monitor.c | 7 +++ > 1 file changed, 7 insertions(+) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v2 16/31] qapi/qom: Add ObjectOptions for confidential-guest-support
Am 24.02.2021 um 16:21 hat Dr. David Alan Gilbert geschrieben:
> * Kevin Wolf (kw...@redhat.com) wrote:
> > This adds a QAPI schema for the properties of the objects implementing
> > the confidential-guest-support interface.
> >
> > pef-guest and s390x-pv-guest don't have any properties, so they only
> > need to be added to the ObjectType enum without adding a new branch to
> > ObjectOptions.
> >
> > Signed-off-by: Kevin Wolf
> > ---
> > qapi/qom.json | 37 +
> > 1 file changed, 37 insertions(+)
> >
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index e7184122e9..d5f68b5c89 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -633,6 +633,38 @@
> >'base': 'RngProperties',
> >'data': { '*filename': 'str' } }
> >
> > +##
> > +# @SevGuestProperties:
> > +#
> > +# Properties for sev-guest objects.
> > +#
> > +# @sev-device: SEV device to use (default: "/dev/sev")
> > +#
> > +# @dh-cert-file: guest owners DH certificate (encoded with base64)
> > +#
> > +# @session-file: guest owners session parameters (encoded with base64)
> > +#
> > +# @policy: SEV policy value (default: 0x1)
> > +#
> > +# @handle: SEV firmware handle (default: 0)
> > +#
> > +# @cbitpos: C-bit location in page table entry (default: 0)
> > +#
> > +# @reduced-phys-bits: number of bits in physical addresses that become
> > +# unavailable when SEV is enabled
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'struct': 'SevGuestProperties',
> > + 'data': { '*sev-device': 'str',
> > +'*dh-cert-file': 'str',
> > +'*session-file': 'str',
> > +'*policy': 'uint32',
> > +'*handle': 'uint32',
> > +'*cbitpos': 'uint32',
> > +'reduced-phys-bits': 'uint32' },
> > + 'if': 'defined(CONFIG_SEV)' }
> > +
> > ##
> > # @ObjectType:
> > #
> > @@ -661,12 +693,15 @@
> > 'memory-backend-file',
> > 'memory-backend-memfd',
> > 'memory-backend-ram',
> > +{'name': 'pef-guest', 'if': 'defined(CONFIG_PSERIES)' },
> > 'pr-manager-helper',
> > 'rng-builtin',
> > 'rng-egd',
> > 'rng-random',
> > 'secret',
> > 'secret_keyring',
> > +{'name': 'sev-guest', 'if': 'defined(CONFIG_SEV)' },
> > +'s390-pv-guest',
>
> If pef-guest is conditional on PSERIES< shouldn't this be dependent on
> s390?
The difference is that s390-pv-guest is compiled unconditionally if the
s390x target is built, whereas CONFIG_PSERIES is a separate thing from
building a ppc target.
I actually tried making it conditional on TARGET_S390X at first, but the
code generated from this schema is supposed to be target independent, so
it rightly failed to build because TARGET_* are marked as poisoned in
most of the codebase.
Kevin
[PATCH 13/33] virfirewall: Don't check OOM in ADD_ARG macro
VIR_RESIZE_N can't fail nowadays, adjust the macro.
Signed-off-by: Peter Krempa
---
src/util/virfirewall.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 6b04a8011f..95962568f5 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -262,10 +262,9 @@ void virFirewallFree(virFirewallPtr firewall)
#define ADD_ARG(rule, str) \
do { \
-if (VIR_RESIZE_N(rule->args, \
- rule->argsAlloc, \
- rule->argsLen, 1) < 0) \
-goto no_memory; \
+ignore_value(VIR_RESIZE_N(rule->args, \
+ rule->argsAlloc, \
+ rule->argsLen, 1)); \
\
rule->args[rule->argsLen++] = g_strdup(str); \
} while (0)
@@ -433,9 +432,6 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
ADD_ARG(rule, arg);
return;
-
- no_memory:
-firewall->err = ENOMEM;
}
@@ -455,9 +451,6 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
ADD_ARG(rule, arg);
return;
-
- no_memory:
-firewall->err = ENOMEM;
}
@@ -473,9 +466,6 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
}
return;
-
- no_memory:
-firewall->err = ENOMEM;
}
@@ -496,10 +486,6 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
va_end(list);
return;
-
- no_memory:
-firewall->err = ENOMEM;
-va_end(list);
}
--
2.29.2
[PATCH 15/33] virfirewall: virFirewallAddRuleFullV: Remove OOM check from VIR_APPEND_ELEMENT
VIR_APPEND_ELEMENT_COPY will abort the program on OOM so there's no need
to check.
Signed-off-by: Peter Krempa
---
src/util/virfirewall.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 66b33d4a91..bbeb87e72d 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -315,24 +315,17 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
ADD_ARG(rule, str);
if (group->addingRollback) {
-if (VIR_APPEND_ELEMENT_COPY(group->rollback,
-group->nrollback,
-rule) < 0)
-goto no_memory;
+ignore_value(VIR_APPEND_ELEMENT_COPY(group->rollback,
+ group->nrollback,
+ rule));
} else {
-if (VIR_APPEND_ELEMENT_COPY(group->action,
-group->naction,
-rule) < 0)
-goto no_memory;
+ignore_value(VIR_APPEND_ELEMENT_COPY(group->action,
+ group->naction,
+ rule));
}
return rule;
-
- no_memory:
-firewall->err = ENOMEM;
-virFirewallRuleFree(rule);
-return NULL;
}
--
2.29.2
[PATCH 18/33] util: xml: Add virXMLBufferCreate wrapper
'xmlBufferCreate' returns NULL only on allocation failure. Add a wrapper
which will call 'abort()' in such case in a centralised spot. It doesn't
make much sense to continue execution from here.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 5 +
src/conf/network_conf.c | 5 +
src/libvirt_private.syms | 1 +
src/util/virxml.c| 19 +--
src/util/virxml.h| 3 +++
src/vmx/vmx.c| 5 ++---
6 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 46620d38ed..7a3374b5be 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28692,10 +28692,7 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr
def,
* Thankfully, libxml maps what looks like globals into
* thread-local uses, so we are thread-safe. */
xmlIndentTreeOutput = 1;
-if (!(xmlbuf = xmlBufferCreate())) {
-virReportOOMError();
-return -1;
-}
+xmlbuf = virXMLBufferCreate();
if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata,
virBufferGetIndent(buf) / 2, 1) < 0) {
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index f32710b781..69d99a60e0 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2513,10 +2513,7 @@ virNetworkDefFormatBuf(virBufferPtr buf,
* Thankfully, libxml maps what looks like globals into
* thread-local uses, so we are thread-safe. */
xmlIndentTreeOutput = 1;
-if (!(xmlbuf = xmlBufferCreate())) {
-virReportOOMError();
-return -1;
-}
+xmlbuf = virXMLBufferCreate();
if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata,
virBufferGetIndent(buf) / 2, 1) < 0) {
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6b7261b987..dd54550b60 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3529,6 +3529,7 @@ virVsockSetGuestCid;
# util/virxml.h
virParseScaledValue;
+virXMLBufferCreate;
virXMLCheckIllegalChars;
virXMLExtractNamespaceXML;
virXMLFormatElement;
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 0354251941..3fed2b2a6e 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -941,12 +941,7 @@ char *
virXMLNodeToString(xmlDocPtr doc,
xmlNodePtr node)
{
-g_autoptr(xmlBuffer) xmlbuf = NULL;
-
-if (!(xmlbuf = xmlBufferCreate())) {
-virReportOOMError();
-return NULL;
-}
+g_autoptr(xmlBuffer) xmlbuf = virXMLBufferCreate();
if (xmlNodeDump(xmlbuf, doc, node, 0, 1) == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1467,3 +1462,15 @@ virParseScaledValue(const char *xpath,
*val = bytes;
return 1;
}
+
+
+xmlBufferPtr
+virXMLBufferCreate(void)
+{
+xmlBufferPtr ret;
+
+if (!(ret = xmlBufferCreate()))
+abort();
+
+return ret;
+}
diff --git a/src/util/virxml.h b/src/util/virxml.h
index e696dd25f5..24a2234506 100644
--- a/src/util/virxml.h
+++ b/src/util/virxml.h
@@ -286,3 +286,6 @@ int virParseScaledValue(const char *xpath,
unsigned long long scale,
unsigned long long max,
bool required);
+
+xmlBufferPtr
+virXMLBufferCreate(void);
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index db535ba260..e6c0900a65 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -771,7 +771,7 @@ virVMXConvertToUTF8(const char *encoding, const char
*string)
char *result = NULL;
xmlCharEncodingHandlerPtr handler;
g_autoptr(xmlBuffer) input = NULL;
-g_autoptr(xmlBuffer) utf8 = NULL;
+g_autoptr(xmlBuffer) utf8 = virXMLBufferCreate();
handler = xmlFindCharEncodingHandler(encoding);
@@ -781,8 +781,7 @@ virVMXConvertToUTF8(const char *encoding, const char
*string)
return NULL;
}
-if (!(input = xmlBufferCreateStatic((char *)string, strlen(string))) ||
-!(utf8 = xmlBufferCreate())) {
+if (!(input = xmlBufferCreateStatic((char *)string, strlen(string {
virReportOOMError();
goto cleanup;
}
--
2.29.2
[PATCH 14/33] virfirewall: Remove OOM checks from virFirewallStartTransaction
Neither virFirewallGroupNew nor VIR_EXPAND_N can fail.
Signed-off-by: Peter Krempa
---
src/util/virfirewall.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 95962568f5..66b33d4a91 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -517,18 +517,11 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
VIR_FIREWALL_RETURN_IF_ERROR(firewall);
-if (!(group = virFirewallGroupNew())) {
-firewall->err = ENOMEM;
-return;
-}
+group = virFirewallGroupNew();
group->actionFlags = flags;
-if (VIR_EXPAND_N(firewall->groups,
- firewall->ngroups, 1) < 0) {
-firewall->err = ENOMEM;
-virFirewallGroupFree(group);
-return;
-}
+ignore_value(VIR_EXPAND_N(firewall->groups,
+ firewall->ngroups, 1));
firewall->groups[firewall->ngroups - 1] = group;
firewall->currentGroup = firewall->ngroups - 1;
}
--
2.29.2
[PATCH 25/33] util: iohelper: Don't handle OOM from posix_memalign
Similarly to other allocation calls abort() on failure.
Signed-off-by: Peter Krempa
---
src/util/iohelper.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 49d939d290..b8810d16d3 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -27,6 +27,7 @@
#include
#include
+#include
#include "virthread.h"
#include "virfile.h"
@@ -57,10 +58,8 @@ runIO(const char *path, int fd, int oflags)
off_t end = 0;
#if WITH_POSIX_MEMALIGN
-if (posix_memalign(, alignMask + 1, buflen)) {
-virReportOOMError();
-goto cleanup;
-}
+if (posix_memalign(, alignMask + 1, buflen))
+abort();
buf = base;
#else
buf = g_new0(char, buflen + alignMask);
--
2.29.2
[PATCH 28/33] libxl: abort() on failure of libxl_cpu_bitmap_alloc()
Attempting to report error in case when we ran out of memory is
pointless.
Signed-off-by: Peter Krempa
---
src/libxl/libxl_conf.c | 6 ++
src/libxl/libxl_driver.c | 7 +++
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f8db118996..f2bcd77a29 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -854,10 +854,8 @@ libxlMakeVnumaList(virDomainDefPtr def,
goto cleanup;
libxl_bitmap_init(_bitmap);
-if (libxl_cpu_bitmap_alloc(ctx, _bitmap, b_info->max_vcpus)) {
-virReportOOMError();
-goto cleanup;
-}
+if (libxl_cpu_bitmap_alloc(ctx, _bitmap, b_info->max_vcpus))
+abort();
do {
libxl_bitmap_set(_bitmap, cpu);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 75a8d46af0..434959d694 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4931,10 +4931,9 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
if (numnodes <= 0)
goto cleanup;
-if (libxl_node_bitmap_alloc(cfg->ctx, , 0)) {
-virReportOOMError();
-goto cleanup;
-}
+if (libxl_node_bitmap_alloc(cfg->ctx, , 0))
+abort();
+
nodes = virBitmapNew(numnodes);
rc = libxl_domain_get_nodeaffinity(cfg->ctx,
--
2.29.2
[PATCH 12/33] virCloseCallbacksGetForConn: Remove OOM handling
VIR_EXPAND_N will abort so we can simplify the hash iterator.
Signed-off-by: Peter Krempa
---
src/hypervisor/virclosecallbacks.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/src/hypervisor/virclosecallbacks.c
b/src/hypervisor/virclosecallbacks.c
index 2641f45a22..1fd4dd7adf 100644
--- a/src/hypervisor/virclosecallbacks.c
+++ b/src/hypervisor/virclosecallbacks.c
@@ -241,7 +241,6 @@ struct _virCloseCallbacksList {
struct virCloseCallbacksData {
virConnectPtr conn;
virCloseCallbacksListPtr list;
-bool oom;
};
static int
@@ -263,11 +262,7 @@ virCloseCallbacksGetOne(void *payload,
if (data->conn != closeDef->conn || !closeDef->cb)
return 0;
-if (VIR_EXPAND_N(data->list->entries,
- data->list->nentries, 1) < 0) {
-data->oom = true;
-return 0;
-}
+ignore_value(VIR_EXPAND_N(data->list->entries, data->list->nentries, 1));
memcpy(data->list->entries[data->list->nentries - 1].uuid,
uuid, VIR_UUID_BUFLEN);
@@ -286,17 +281,9 @@ virCloseCallbacksGetForConn(virCloseCallbacksPtr
closeCallbacks,
data.conn = conn;
data.list = list;
-data.oom = false;
virHashForEach(closeCallbacks->list, virCloseCallbacksGetOne, );
-if (data.oom) {
-VIR_FREE(list->entries);
-VIR_FREE(list);
-virReportOOMError();
-return NULL;
-}
-
return list;
}
--
2.29.2
[PATCH 22/33] virXMLParseHelper: abort() on allocation failure
Signed-off-by: Peter Krempa
---
src/util/virxml.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 62bbafacd6..060b7530fc 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -789,10 +789,8 @@ virXMLParseHelper(int domcode,
/* Set up a parser context so we can catch the details of XML errors. */
pctxt = xmlNewParserCtxt();
-if (!pctxt || !pctxt->sax) {
-virReportOOMError();
-goto error;
-}
+if (!pctxt || !pctxt->sax)
+abort();
private.domcode = domcode;
pctxt->_private =
--
2.29.2
[PATCH 21/33] virXMLXPathContextNew: abort() on allocation failure
Signed-off-by: Peter Krempa
---
src/util/virxml.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index ebe479f5d3..62bbafacd6 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -51,10 +51,8 @@ virXMLXPathContextNew(xmlDocPtr xml)
{
xmlXPathContextPtr ctxt;
-if (!(ctxt = xmlXPathNewContext(xml))) {
-virReportOOMError();
-return NULL;
-}
+if (!(ctxt = xmlXPathNewContext(xml)))
+abort();
return ctxt;
}
--
2.29.2
[PATCH 26/33] hyperv: abort() failure of wsmc_fault_new()
The function just allocates a helper object. Reporting errors would be
pointless when we encounter OOM situation.
Signed-off-by: Peter Krempa
---
src/hyperv/hyperv_wmi.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 8bb6f591f1..2a898cdf03 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -92,12 +92,8 @@ hypervVerifyResponse(WsManClient *client, WsXmlDocH response,
}
if (wsmc_check_for_fault(response)) {
-fault = wsmc_fault_new();
-
-if (fault == NULL) {
-virReportOOMError();
-return -1;
-}
+if (!(fault = wsmc_fault_new()))
+abort();
wsmc_get_fault_data(response, fault);
--
2.29.2
[PATCH 31/33] storage: Don't report OOM error on failure of glfs_new
OOM isn't the only failure glfs_new can encounter. Report an error which
might give more insight. libgfapi seems to be setting errno but
reporting a system error migt be misleading.
Signed-off-by: Peter Krempa
---
src/storage/storage_backend_gluster.c | 3 ++-
src/storage_file/storage_file_backend_gluster.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 8de0cb8a6b..e673922df6 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -114,7 +114,8 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool)
/* Actually connect to glfs */
if (!(ret->vol = glfs_new(ret->volname))) {
-virReportOOMError();
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _("failed to create glfs object for '%s'"),
ret->volname);
goto error;
}
diff --git a/src/storage_file/storage_file_backend_gluster.c
b/src/storage_file/storage_file_backend_gluster.c
index 9b3b783274..8c7a583886 100644
--- a/src/storage_file/storage_file_backend_gluster.c
+++ b/src/storage_file/storage_file_backend_gluster.c
@@ -120,7 +120,8 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
(unsigned int)drv->uid, (unsigned int)drv->gid);
if (!(priv->vol = glfs_new(src->volume))) {
-virReportOOMError();
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _("failed to create glfs object for '%s'"),
src->volume);
goto error;
}
--
2.29.2
[PATCH 32/33] virVMXConvertToUTF8: Report non-OOM error on failure of xmlBufferCreateStatic
The function has also non-OOM failure case when the passed string has 0
length, so reporting OOM error is not correct.
Signed-off-by: Peter Krempa
---
src/vmx/vmx.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index e6c0900a65..73bf7c4f3d 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -781,12 +781,8 @@ virVMXConvertToUTF8(const char *encoding, const char
*string)
return NULL;
}
-if (!(input = xmlBufferCreateStatic((char *)string, strlen(string {
-virReportOOMError();
-goto cleanup;
-}
-
-if (xmlCharEncInFunc(handler, utf8, input) < 0) {
+if (!(input = xmlBufferCreateStatic((char *)string, strlen(string))) ||
+xmlCharEncInFunc(handler, utf8, input) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not convert from %s to UTF-8 encoding"),
encoding);
goto cleanup;
--
2.29.2
[PATCH 33/33] util: virerror: Remove virReportOOMError
Trying to report an OOM error is pointless since our infrastructure to
report error needs to allocate memory to report the error.
In addition our code mistakenly reported OOM errors even in cases where
a function could fail for another reason, which would make issues harder
to debug.
Remove the virReportOOMError and backend so that programmers are forced
to think about what can happen. In case when there's another failure
possible a specific error should be reported and otherwise a direct
abort() is better since the logger would abort on g_new anyways.
This patch also removes the syntas-check which forces use of
virReportOOMError instead of using VIR_ERR_NO_MEMORY with other
functions. This allows possible future use when we'd end up in a
situation where trying to recover from an OOM would make sense, such as
when attempting to allocate a massive buffer.
Signed-off-by: Peter Krempa
---
build-aux/syntax-check.mk | 8
src/libvirt_private.syms | 1 -
src/util/virerror.c | 22 --
src/util/virerror.h | 8
4 files changed, 39 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index e51877648a..e1ccb74986 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -490,11 +490,6 @@ sc_prohibit_gettext_noop:
halt='use N_, not gettext_noop' \
$(_sc_search_regexp)
-sc_prohibit_VIR_ERR_NO_MEMORY:
- @prohibit='\' \
- halt='use virReportOOMError, not VIR_ERR_NO_MEMORY' \
- $(_sc_search_regexp)
-
sc_prohibit_PATH_MAX:
@prohibit='\' \
halt='dynamically allocate paths, do not use PATH_MAX' \
@@ -1895,9 +1890,6 @@ exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics
= \
exclude_file_name_regexp--sc_po_check =
^(docs/|src/rpc/gendispatch\.pl$$|tests/commandtest.c$$)
-exclude_file_name_regexp--sc_prohibit_VIR_ERR_NO_MEMORY = \
-
^(build-aux/syntax-check\.mk|include/libvirt/virterror\.h|src/remote/remote_daemon_dispatch\.c|src/util/virerror\.c|docs/internals/oomtesting\.html\.in)$$
-
exclude_file_name_regexp--sc_prohibit_PATH_MAX = \
^build-aux/syntax-check\.mk$$
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 48f66daab8..7eb37ed797 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2116,7 +2116,6 @@ virLastErrorPrefixMessage;
virRaiseErrorFull;
virRaiseErrorObject;
virReportErrorHelper;
-virReportOOMErrorFull;
virReportSystemErrorFull;
virSetError;
virSetErrorLogPriorityFunc;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 708081414a..a503cdefdc 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1358,28 +1358,6 @@ void virReportSystemErrorFull(int domcode,
errno = save_errno;
}
-/**
- * virReportOOMErrorFull:
- * @domcode: the virErrorDomain indicating where it's coming from
- * @filename: filename where error was raised
- * @funcname: function name where error was raised
- * @linenr: line number where error was raised
- *
- * Convenience internal routine called when an out of memory error is
- * detected
- */
-void virReportOOMErrorFull(int domcode,
- const char *filename,
- const char *funcname,
- size_t linenr)
-{
-const char *virerr;
-
-virerr = virErrorMsg(VIR_ERR_NO_MEMORY, NULL);
-virRaiseErrorFull(filename, funcname, linenr,
- domcode, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
- virerr, NULL, NULL, -1, -1, virerr, NULL);
-}
/**
* virSetErrorLogPriorityFunc:
diff --git a/src/util/virerror.h b/src/util/virerror.h
index 9d3e40d65a..da7d7c0afe 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -174,14 +174,6 @@ void virReportSystemErrorFull(int domcode,
"Unexpected enum value %d for %s", \
value, sizeof((typname)1) != 0 ? #typname : #typname);
-void virReportOOMErrorFull(int domcode,
- const char *filename,
- const char *funcname,
- size_t linenr);
-
-#define virReportOOMError() \
-virReportOOMErrorFull(VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__)
-
#define virReportError(code, ...) \
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
--
2.29.2
[PATCH 29/33] virVBoxSnapshotConfSaveVboxFile: abort() on failure to allocate xmlDoc and comment
'xmlNewDoc' and 'xmlNewDocComment' return NULL only on allocation
failure. Attempting to raise an error is pointless.
Signed-off-by: Peter Krempa
---
src/vbox/vbox_snapshot_conf.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
index 5792d3175e..4f12d2ebfa 100644
--- a/src/vbox/vbox_snapshot_conf.c
+++ b/src/vbox/vbox_snapshot_conf.c
@@ -996,10 +996,8 @@
virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachinePtr machine,
goto cleanup;
}
xml = xmlNewDoc(BAD_CAST "1.0");
-if (!xml) {
-virReportOOMError();
-goto cleanup;
-}
+if (!xml)
+abort();
cur = virXMLNewNode(NULL, "VirtualBox");
@@ -1023,10 +1021,8 @@
virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachinePtr machine,
"OVERWRITTEN AND LOST.\n"
"Changes to this xml configuration should be made
using Virtualbox\n"
"or other application using the libvirt API");
-if (!cur) {
-virReportOOMError();
-goto cleanup;
-}
+if (!cur)
+abort();
if (!xmlAddPrevSibling(xmlDocGetRootElement(xml), cur)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
--
2.29.2
[PATCH 30/33] util: json: Report non-OOM error on yajl failure
The yajl library returns a wide range of error codes so reporting OOM on
any failure is wrong. In case the error was really based by memory issue
the error reporting will probably cause an abort anyways. Change the
error message so that we know that it happened in JSON at least.
Signed-off-by: Peter Krempa
---
src/util/virjson.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index e74b9fca4f..f2a6024db6 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1943,12 +1943,14 @@ virJSONValueToBuffer(virJSONValuePtr object,
yajl_gen_config(g, yajl_gen_validate_utf8, 1);
if (virJSONValueToStringOne(object, g) < 0) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to convert virJSONValue to yajl data"));
goto cleanup;
}
if (yajl_gen_get_buf(g, , ) != yajl_gen_status_ok) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to format JSON"));
goto cleanup;
}
--
2.29.2
[PATCH 23/33] util: virprocess: abort() on CPU_ALLOC failure
Signed-off-by: Peter Krempa
---
src/util/virprocess.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 41c5678537..69d64e9466 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -462,10 +462,8 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map,
bool quiet)
masklen = CPU_ALLOC_SIZE(numcpus);
mask = CPU_ALLOC(numcpus);
-if (!mask) {
-virReportOOMError();
-return -1;
-}
+if (!mask)
+abort();
CPU_ZERO_S(masklen, mask);
for (i = 0; i < virBitmapSize(map); i++) {
@@ -509,10 +507,8 @@ virProcessGetAffinity(pid_t pid)
masklen = CPU_ALLOC_SIZE(ncpus);
mask = CPU_ALLOC(ncpus);
-if (!mask) {
-virReportOOMError();
-return NULL;
-}
+if (!mask)
+abort();
CPU_ZERO_S(masklen, mask);
--
2.29.2
[PATCH 20/33] Don't report OOM error on xmlCopyNode failure
Out of memory isn't the only reason the function can fail. Add a message
stating that copying of a XML node failed.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 3 ++-
src/test/test_driver.c | 6 --
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c0881608af..f1fd5320a5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30460,7 +30460,8 @@ virDomainDefSetMetadata(virDomainDefPtr def,
def->metadata = virXMLNewNode(NULL, "metadata");
if (!(new = xmlCopyNode(doc->children, 1))) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to copy XML node"));
return -1;
}
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index bca1297d1d..71ab04aa1a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -224,7 +224,8 @@ testDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
for (i = 0; i < n; i++) {
xmlNodePtr newnode = xmlCopyNode(nodes[i], 1);
if (!newnode) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to copy XML node"));
goto error;
}
@@ -787,7 +788,8 @@ testParseXMLDocFromFile(xmlNodePtr node, const char *file,
const char *type)
ret = xmlCopyNode(xmlDocGetRootElement(doc), 1);
if (!ret) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to copy XML node"));
goto error;
}
xmlReplaceNode(node, ret);
--
2.29.2
[PATCH 17/33] util: virnetlink: Add wrapper for 'nlmsg_alloc_simple'
The function is used in many places and fails only on allocation
failures. Since trying to recover from allocation failure of a small
buffer by reporting error doesn't make sense add a wrapper for
'nlmsg_alloc_simple' which will 'abort()' on failure and replace all
allocations of netlink message with the new helper.
Signed-off-by: Peter Krempa
---
src/util/virnetdev.c | 18 +++
src/util/virnetdevbridge.c | 10 +++--
src/util/virnetdevip.c | 15 -
src/util/virnetdevvportprofile.c | 6 +
src/util/virnetlink.c| 38 ++--
src/util/virnetlink.h| 4
6 files changed, 32 insertions(+), 59 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 2b4c8b6280..d0c76ce26c 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1596,11 +1596,7 @@ virNetDevSetVfConfig(const char *ifname, int vf,
if (!macaddr && vlanid < 0)
return -1;
-nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST);
-if (!nl_msg) {
-virReportOOMError();
-return rc;
-}
+nl_msg = virNetlinkMsgNew(RTM_SETLINK, NLM_F_REQUEST);
if (nlmsg_append(nl_msg, , sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
goto buffer_too_small;
@@ -3132,11 +3128,7 @@ virNetDevGetFamilyId(const char *family_name,
unsigned int recvbuflen;
int ret = -1;
-if (!(nl_msg = nlmsg_alloc_simple(GENL_ID_CTRL,
- NLM_F_REQUEST | NLM_F_ACK))) {
-virReportOOMError();
-goto cleanup;
-}
+nl_msg = virNetlinkMsgNew(GENL_ID_CTRL, NLM_F_REQUEST | NLM_F_ACK);
if (nlmsg_append(nl_msg, , sizeof(gmsgh), NLMSG_ALIGNTO) < 0)
goto cleanup;
@@ -3220,11 +3212,7 @@ virNetDevSwitchdevFeature(const char *ifname,
if ((rv = virNetDevGetFamilyId(DEVLINK_GENL_NAME, _id)) <= 0)
return rv;
-if (!(nl_msg = nlmsg_alloc_simple(family_id,
- NLM_F_REQUEST | NLM_F_ACK))) {
-virReportOOMError();
-goto cleanup;
-}
+nl_msg = virNetlinkMsgNew(family_id, NLM_F_REQUEST | NLM_F_ACK);
if (nlmsg_append(nl_msg, , sizeof(gmsgh), NLMSG_ALIGNTO) < 0)
goto cleanup;
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index d475e4c43d..7b5ea4fe1d 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -1036,13 +1036,9 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const
char *ifname,
if (!(ndm.ndm_state & (NUD_PERMANENT | NUD_REACHABLE)))
ndm.ndm_state |= NUD_PERMANENT;
-nl_msg = nlmsg_alloc_simple(isAdd ? RTM_NEWNEIGH : RTM_DELNEIGH,
-NLM_F_REQUEST |
-(isAdd ? (NLM_F_CREATE | NLM_F_EXCL) : 0));
-if (!nl_msg) {
-virReportOOMError();
-return -1;
-}
+nl_msg = virNetlinkMsgNew(isAdd ? RTM_NEWNEIGH : RTM_DELNEIGH,
+ NLM_F_REQUEST |
+ (isAdd ? (NLM_F_CREATE | NLM_F_EXCL) : 0));
if (nlmsg_append(nl_msg, , sizeof(ndm), NLMSG_ALIGNTO) < 0)
goto buffer_too_small;
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 4eb8ef76d1..83da7bc46d 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -107,11 +107,8 @@ virNetDevCreateNetlinkAddressMessage(int messageType,
if ((ifindex = if_nametoindex(ifname)) == 0)
return NULL;
-if (!(nlmsg = nlmsg_alloc_simple(messageType,
- NLM_F_REQUEST | NLM_F_CREATE |
NLM_F_EXCL))) {
-virReportOOMError();
-return NULL;
-}
+nlmsg = virNetlinkMsgNew(messageType,
+ NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
memset(, 0, sizeof(ifa));
@@ -323,12 +320,8 @@ virNetDevIPRouteAdd(const char *ifname,
if ((ifindex = if_nametoindex(ifname)) == 0)
return -1;
-if (!(nlmsg = nlmsg_alloc_simple(RTM_NEWROUTE,
- NLM_F_REQUEST | NLM_F_CREATE |
- NLM_F_EXCL))) {
-virReportOOMError();
-return -1;
-}
+nlmsg = virNetlinkMsgNew(RTM_NEWROUTE,
+ NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
memset(, 0, sizeof(rtmsg));
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 5d6c055b32..c0fc04c699 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -689,11 +689,7 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int
ifindex,
? virUUIDFormat(hostUUID, hostUUIDStr)
: "(unspecified)"));
-nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST);
-if (!nl_msg) {
-virReportOOMError();
-return rc;
-}
+nl_msg = virNetlinkMsgNew(RTM_SETLINK, NLM_F_REQUEST);
if (nlmsg_append(nl_msg, , sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
[PATCH 27/33] vbox: abort() on allocation failure in UTF8<->UTF16 conversion
Trying to report an error on OOM is pointless since error handling
allocates memory.
Signed-off-by: Peter Krempa
---
src/vbox/vbox_common.c | 20
src/vbox/vbox_common.h | 15 +--
2 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 138403b034..ce9bfbf827 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -5465,17 +5465,9 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
}
VBOX_UTF8_TO_UTF16(def->parent.name, );
-if (!name) {
-virReportOOMError();
-goto cleanup;
-}
if (def->parent.description) {
VBOX_UTF8_TO_UTF16(def->parent.description, );
-if (!description) {
-virReportOOMError();
-goto cleanup;
-}
}
rc = gVBoxAPI.UIConsole.TakeSnapshot(console, name, description,
);
@@ -6475,10 +6467,6 @@ vboxDomainSnapshotGetParent(virDomainSnapshotPtr
snapshot,
goto cleanup;
}
VBOX_UTF16_TO_UTF8(nameUtf16, );
-if (!name) {
-virReportOOMError();
-goto cleanup;
-}
ret = virGetDomainSnapshot(dom, name);
@@ -6533,10 +6521,6 @@ vboxDomainSnapshotCurrent(virDomainPtr dom, unsigned int
flags)
}
VBOX_UTF16_TO_UTF8(nameUtf16, );
-if (!name) {
-virReportOOMError();
-goto cleanup;
-}
ret = virGetDomainSnapshot(dom, name);
@@ -6593,10 +6577,6 @@ static int
vboxDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
}
VBOX_UTF16_TO_UTF8(nameUtf16, );
-if (!name) {
-virReportOOMError();
-goto cleanup;
-}
ret = STREQ(snapshot->name, name);
diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
index 8b1fb2ac30..1fb922aaf0 100644
--- a/src/vbox/vbox_common.h
+++ b/src/vbox/vbox_common.h
@@ -391,8 +391,19 @@ typedef nsISupports IKeyboard;
} \
} while (0)
-#define VBOX_UTF16_TO_UTF8(arg1, arg2)
gVBoxAPI.UPFN.Utf16ToUtf8(data->pFuncs, arg1, arg2)
-#define VBOX_UTF8_TO_UTF16(arg1, arg2)
gVBoxAPI.UPFN.Utf8ToUtf16(data->pFuncs, arg1, arg2)
+#define VBOX_UTF16_TO_UTF8(arg1, arg2) \
+do { \
+gVBoxAPI.UPFN.Utf16ToUtf8(data->pFuncs, arg1, arg2); \
+if (!*(arg2)) \
+abort(); \
+} while (0)
+
+#define VBOX_UTF8_TO_UTF16(arg1, arg2) \
+do { \
+gVBoxAPI.UPFN.Utf8ToUtf16(data->pFuncs, arg1, arg2); \
+if (!*(arg2)) \
+abort(); \
+} while (0)
#define VBOX_ADDREF(arg)gVBoxAPI.nsUISupports.AddRef((void
*)(arg))
--
2.29.2
[PATCH 24/33] virURIFormat: abort() on failure
If the argument of 'xmlSaveUri' is non-NULL the function returns NULL on
OOM failure only. Thus we can directly abort rather than try to do the
impossible recovery.
Signed-off-by: Peter Krempa
---
src/util/viruri.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 0aafd49d6d..1e8808ddc6 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -238,11 +238,9 @@ virURIFormat(virURIPtr uri)
if (!xmluri.server && !xmluri.port)
xmluri.port = -1;
-ret = (char *)xmlSaveUri();
-if (!ret) {
-virReportOOMError();
-return NULL;
-}
+/* xmlSaveUri can fail only on OOM condition if argument is non-NULL */
+if (!(ret = (char *)xmlSaveUri()))
+abort();
return ret;
}
--
2.29.2
[PATCH 19/33] util: xml: Add wrapper for 'xmlNewNode'
Add a wrapper that will handle the out of memory condition by abort()
and also prevents callers from having to typecast the argument.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c| 7 ++-
src/libvirt_private.syms | 1 +
src/util/virxml.c | 13 +
src/util/virxml.h | 4
src/vbox/vbox_snapshot_conf.c | 34 +-
tools/virsh-domain.c | 5 +
6 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7a3374b5be..c0881608af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30456,11 +30456,8 @@ virDomainDefSetMetadata(virDomainDefPtr def,
return -1;
/* create the root node if needed */
-if (!def->metadata &&
-!(def->metadata = xmlNewNode(NULL, (unsigned char
*)"metadata"))) {
-virReportOOMError();
-return -1;
-}
+if (!def->metadata)
+def->metadata = virXMLNewNode(NULL, "metadata");
if (!(new = xmlCopyNode(doc->children, 1))) {
virReportOOMError();
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dd54550b60..48f66daab8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3533,6 +3533,7 @@ virXMLBufferCreate;
virXMLCheckIllegalChars;
virXMLExtractNamespaceXML;
virXMLFormatElement;
+virXMLNewNode;
virXMLNodeContentString;
virXMLNodeNameEqual;
virXMLNodeSanitizeNamespaces;
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 3fed2b2a6e..ebe479f5d3 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -1474,3 +1474,16 @@ virXMLBufferCreate(void)
return ret;
}
+
+
+xmlNodePtr
+virXMLNewNode(xmlNsPtr ns,
+ const char *name)
+{
+xmlNodePtr ret;
+
+if (!(ret = xmlNewNode(ns, BAD_CAST name)))
+abort();
+
+return ret;
+}
diff --git a/src/util/virxml.h b/src/util/virxml.h
index 24a2234506..d32f77b867 100644
--- a/src/util/virxml.h
+++ b/src/util/virxml.h
@@ -289,3 +289,7 @@ int virParseScaledValue(const char *xpath,
xmlBufferPtr
virXMLBufferCreate(void);
+
+xmlNodePtr
+virXMLNewNode(xmlNsPtr ns,
+ const char *name);
diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
index f1cae3039a..5792d3175e 100644
--- a/src/vbox/vbox_snapshot_conf.c
+++ b/src/vbox/vbox_snapshot_conf.c
@@ -328,7 +328,7 @@
virVBoxSnapshotConfCreateHardDiskNode(virVBoxSnapshotConfHardDiskPtr hardDisk)
int result = -1;
size_t i = 0;
char *uuid = NULL;
-xmlNodePtr ret = xmlNewNode(NULL, BAD_CAST "HardDisk");
+xmlNodePtr ret = virXMLNewNode(NULL, "HardDisk");
uuid = g_strdup_printf("{%s}", hardDisk->uuid);
if (xmlNewProp(ret, BAD_CAST "uuid", BAD_CAST uuid) == NULL)
@@ -404,7 +404,7 @@ virVBoxSnapshotConfSerializeSnapshot(xmlNodePtr node,
/* node description */
if (snapshot->description != NULL) {
-descriptionNode = xmlNewNode(NULL, BAD_CAST "Description");
+descriptionNode = virXMLNewNode(NULL, "Description");
xmlNodeSetContent(descriptionNode, BAD_CAST snapshot->description);
xmlAddChild(node, descriptionNode);
}
@@ -433,10 +433,10 @@ virVBoxSnapshotConfSerializeSnapshot(xmlNodePtr node,
xmlAddChild(node, storageControllerNode);
if (snapshot->nchildren > 0) {
-snapshotsNode = xmlNewNode(NULL, BAD_CAST "Snapshots");
+snapshotsNode = virXMLNewNode(NULL, "Snapshots");
xmlAddChild(node, snapshotsNode);
for (i = 0; i < snapshot->nchildren; i++) {
-xmlNodePtr child = xmlNewNode(NULL, BAD_CAST "Snapshot");
+xmlNodePtr child = virXMLNewNode(NULL, "Snapshot");
xmlAddChild(snapshotsNode, child);
if (virVBoxSnapshotConfSerializeSnapshot(child,
snapshot->children[i]) < 0)
goto cleanup;
@@ -1001,11 +1001,7 @@
virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachinePtr machine,
goto cleanup;
}
-cur = xmlNewNode(NULL, BAD_CAST "VirtualBox");
-if (!cur) {
-virReportOOMError();
-goto cleanup;
-}
+cur = virXMLNewNode(NULL, "VirtualBox");
if (!xmlNewProp(cur, BAD_CAST "version", BAD_CAST "1.12-linux")) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1038,11 +1034,7 @@
virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachinePtr machine,
goto cleanup;
}
-machineNode = xmlNewNode(NULL, BAD_CAST "Machine");
-if (!machineNode) {
-virReportOOMError();
-goto cleanup;
-}
+machineNode = virXMLNewNode(NULL, "Machine");
if (!xmlNewProp(machineNode, BAD_CAST "uuid", BAD_CAST machine->uuid)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1101,11 +1093,7 @@
virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachinePtr machine,
}
xmlAddChild(xmlDocGetRootElement(xml),
[PATCH 16/33] virfirewall: Remove impossible OOM error reporting
There's nothing that would set the 'err' field of virFirewallPtr to
ENOMEM so we can remove the checks.
Signed-off-by: Peter Krempa
---
src/util/virfirewall.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index bbeb87e72d..c1b7d2268b 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -698,10 +698,6 @@ virFirewallApplyRule(virFirewallPtr firewall,
if (rule->queryCB(firewall, rule->layer, (const char *const *)lines,
rule->queryOpaque) < 0)
return -1;
-if (firewall->err == ENOMEM) {
-virReportOOMError();
-return -1;
-}
if (firewall->err) {
virReportSystemError(firewall->err, "%s",
_("Unable to create rule"));
@@ -769,11 +765,7 @@ virFirewallApply(virFirewallPtr firewall)
_("Failed to initialize a valid firewall backend"));
goto cleanup;
}
-if (!firewall || firewall->err == ENOMEM) {
-virReportOOMError();
-goto cleanup;
-}
-if (firewall->err) {
+if (!firewall || firewall->err) {
virReportSystemError(firewall->err, "%s",
_("Unable to create rule"));
goto cleanup;
--
2.29.2
[PATCH 11/33] util: vircommand: Remove OOM handling
The OOM error handling is dead code nowadays.
Signed-off-by: Peter Krempa
---
src/util/vircommand.c | 96 +--
1 file changed, 20 insertions(+), 76 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index f11caf0d6e..1a4b77ea24 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -90,7 +90,7 @@ struct _virCommandSendBuffer {
};
struct _virCommand {
-int has_error; /* ENOMEM on allocation failure, -1 for anything else. */
+int has_error; /* 0 on success, -1 on error */
char **args;
size_t nargs;
@@ -198,7 +198,6 @@ virCommandFDIsSet(virCommandPtr cmd,
*
* Returns: 0 on success,
* -1 on usage error,
- * ENOMEM on OOM
*/
static int
virCommandFDSet(virCommandPtr cmd,
@@ -211,8 +210,7 @@ virCommandFDSet(virCommandPtr cmd,
if (virCommandFDIsSet(cmd, fd))
return 0;
-if (VIR_EXPAND_N(cmd->passfd, cmd->npassfd, 1) < 0)
-return ENOMEM;
+ignore_value(VIR_EXPAND_N(cmd->passfd, cmd->npassfd, 1));
cmd->passfd[cmd->npassfd - 1].fd = fd;
cmd->passfd[cmd->npassfd - 1].flags = flags;
@@ -1344,10 +1342,7 @@ virCommandAddEnv(virCommandPtr cmd,
}
/* Arg plus trailing NULL. */
-if (VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 1 + 1) < 0) {
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 1 + 1));
cmd->env[cmd->nenv++] = g_steal_pointer();
}
@@ -1474,10 +1469,7 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
if (!cmd || cmd->has_error)
return;
-if (VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 9) < 0) {
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 9));
virCommandAddEnvPair(cmd, "LC_ALL", "C");
@@ -1497,10 +1489,7 @@ virCommandAddEnvXDG(virCommandPtr cmd, const char
*baseDir)
if (!cmd || cmd->has_error)
return;
-if (VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 3) < 0) {
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 3));
virCommandAddEnvFormat(cmd, "XDG_DATA_HOME=%s/%s",
baseDir, ".local/share");
@@ -1530,10 +1519,7 @@ virCommandAddArg(virCommandPtr cmd, const char *val)
}
/* Arg plus trailing NULL. */
-if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1));
cmd->args[cmd->nargs++] = g_strdup(val);
}
@@ -1559,10 +1545,7 @@ virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr
buf)
str = g_strdup("");
/* Arg plus trailing NULL. */
-if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1));
cmd->args[cmd->nargs] = g_steal_pointer();
cmd->nargs++;
@@ -1591,11 +1574,7 @@ virCommandAddArgFormat(virCommandPtr cmd, const char
*format, ...)
va_end(list);
/* Arg plus trailing NULL. */
-if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
-VIR_FREE(arg);
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1));
cmd->args[cmd->nargs++] = arg;
}
@@ -1642,10 +1621,7 @@ virCommandAddArgSet(virCommandPtr cmd, const char
*const*vals)
narg++;
/* narg plus trailing NULL. */
-if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, narg + 1) < 0) {
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, narg + 1));
narg = 0;
while (vals[narg] != NULL) {
@@ -1678,10 +1654,7 @@ virCommandAddArgList(virCommandPtr cmd, ...)
va_end(list);
/* narg plus trailing NULL. */
-if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, narg + 1) < 0) {
-cmd->has_error = ENOMEM;
-return;
-}
+ignore_value(VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, narg + 1));
va_start(list, cmd);
while (1) {
@@ -1765,10 +1738,7 @@ virCommandSetSendBuffer(virCommandPtr cmd,
}
i = virCommandGetNumSendBuffers(cmd);
-if (VIR_REALLOC_N(cmd->sendBuffers, i + 1) < 0) {
-cmd->has_error = ENOMEM;
-return -1;
-}
+ignore_value(VIR_REALLOC_N(cmd->sendBuffers, i + 1));
cmd->sendBuffers[i].fd = fd;
cmd->sendBuffers[i].buffer = buffer;
@@ -2099,11 +2069,7 @@ virCommandToString(virCommandPtr cmd, bool linebreaks)
/* Cannot assume virCommandRun will be called; so report the error
* now. If virCommandRun is called, it will report the same error. */
-if (!cmd ||cmd->has_error == ENOMEM) {
-virReportOOMError();
-
[PATCH 10/33] virDomainDefSetMetadata: Rework memory handling
Switch to use g_autoptr for 'doc' and 'new' local variables.
Additionally report proper error when 'xmlAddChild' fails because OOM is
not the only error it can report.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 34 --
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6f4487fcfc..46620d38ed 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30423,15 +30423,14 @@ virDomainDefSetMetadata(virDomainDefPtr def,
const char *key,
const char *uri)
{
-xmlDocPtr doc = NULL;
+g_autoptr(xmlDoc) doc = NULL;
xmlNodePtr old;
-xmlNodePtr new = NULL;
-int ret = -1;
+g_autoptr(xmlNode) new = NULL;
if (type >= VIR_DOMAIN_METADATA_LAST) {
virReportError(VIR_ERR_INVALID_ARG,
_("unknown metadata type '%d'"), type);
-goto cleanup;
+return -1;
}
switch ((virDomainMetadataType) type) {
@@ -30451,23 +30450,24 @@ virDomainDefSetMetadata(virDomainDefPtr def,
case VIR_DOMAIN_METADATA_ELEMENT:
if (metadata) {
+
/* parse and modify the xml from the user */
if (!(doc = virXMLParseString(metadata, _("(metadata_xml)"
-goto cleanup;
+return -1;
if (virXMLInjectNamespace(doc->children, uri, key) < 0)
-goto cleanup;
+return -1;
/* create the root node if needed */
if (!def->metadata &&
!(def->metadata = xmlNewNode(NULL, (unsigned char
*)"metadata"))) {
virReportOOMError();
-goto cleanup;
+return -1;
}
if (!(new = xmlCopyNode(doc->children, 1))) {
virReportOOMError();
-goto cleanup;
+return -1;
}
}
@@ -30477,11 +30477,13 @@ virDomainDefSetMetadata(virDomainDefPtr def,
xmlFreeNode(old);
}
-if (new &&
-!(xmlAddChild(def->metadata, new))) {
-xmlFreeNode(new);
-virReportOOMError();
-goto cleanup;
+if (new) {
+if (!(xmlAddChild(def->metadata, new))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to add metadata to XML document"));
+return -1;
+}
+new = NULL;
}
break;
@@ -30490,11 +30492,7 @@ virDomainDefSetMetadata(virDomainDefPtr def,
break;
}
-ret = 0;
-
- cleanup:
-xmlFreeDoc(doc);
-return ret;
+return 0;
}
--
2.29.2
[PATCH 09/33] lxc_process: Remove OOM handling from logging setup
'virLogGetFilters' doesn't return failure and 'virLogGetOutputs' reports
it's own errors.
Signed-off-by: Peter Krempa
---
src/lxc/lxc_process.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index cbc04a3dcd..679709605e 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -960,21 +960,14 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
if (virLogGetNbFilters() > 0) {
filterstr = virLogGetFilters();
-if (!filterstr) {
-virReportOOMError();
-goto error;
-}
virCommandAddEnvPair(cmd, "LIBVIRT_LOG_FILTERS", filterstr);
}
if (cfg->log_libvirtd) {
if (virLogGetNbOutputs() > 0) {
-outputstr = virLogGetOutputs();
-if (!outputstr) {
-virReportOOMError();
+if (!(outputstr = virLogGetOutputs()))
goto error;
-}
virCommandAddEnvPair(cmd, "LIBVIRT_LOG_OUTPUTS", outputstr);
}
--
2.29.2
[PATCH 05/33] virCommandAddArgBuffer: Simplify clearing of @buf
Get the buffer contents into a temporary variable with automatic
clearing so that the error branches don't have to reset the buffer.
Additionally handle the NULL string case before assignment.
Signed-off-by: Peter Krempa
---
src/util/vircommand.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index b94ab615d5..f11caf0d6e 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1550,21 +1550,21 @@ virCommandAddArg(virCommandPtr cmd, const char *val)
void
virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr buf)
{
-if (!cmd || cmd->has_error) {
-virBufferFreeAndReset(buf);
+g_autofree char *str = virBufferContentAndReset(buf);
+
+if (!cmd || cmd->has_error)
return;
-}
+
+if (!str)
+str = g_strdup("");
/* Arg plus trailing NULL. */
if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
cmd->has_error = ENOMEM;
-virBufferFreeAndReset(buf);
return;
}
-cmd->args[cmd->nargs] = virBufferContentAndReset(buf);
-if (!cmd->args[cmd->nargs])
-cmd->args[cmd->nargs] = g_strdup("");
+cmd->args[cmd->nargs] = g_steal_pointer();
cmd->nargs++;
}
--
2.29.2
[PATCH 06/33] virCPUx86DataParse: Don't check error from x86FeatureNames
x86FeatureNames uses virBuffer and thus can't fail nowadays.
Signed-off-by: Peter Krempa
---
src/cpu/cpu_x86.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 92f945beb4..79c5868ae6 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1856,11 +1856,7 @@ virCPUx86DataParse(xmlXPathContextPtr ctxt)
*/
#define virX86CpuIncompatible(MSG, CPU_DEF) \
do { \
-g_autofree char *flagsStr = NULL; \
-if (!(flagsStr = x86FeatureNames(map, ", ", (CPU_DEF { \
-virReportOOMError(); \
-return VIR_CPU_COMPARE_ERROR; \
-} \
+g_autofree char *flagsStr = x86FeatureNames(map, ", ", (CPU_DEF));
\
if (message) \
*message = g_strdup_printf("%s: %s", _(MSG), flagsStr); \
VIR_DEBUG("%s: %s", MSG, flagsStr); \
--
2.29.2
[PATCH 08/33] virBuildPath: Remove return value
The function can't fail nowadays, remove the return value and adjust
callers.
Signed-off-by: Peter Krempa
---
docs/internals/command.html.in | 8 +---
src/util/virfcp.c | 3 +--
src/util/virfile.c | 7 +--
src/util/virfile.h | 2 +-
src/util/virhook.c | 14 ++
src/util/virpci.c | 11 ++-
6 files changed, 8 insertions(+), 37 deletions(-)
diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index 823d89cc71..634afdc937 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -568,13 +568,7 @@ int runhook(const char *drvstr, const char *id,
char *path;
virCommandPtr cmd;
- ret = virBuildPath(path, LIBVIRT_HOOK_DIR, drvstr);
- if ((ret 0) || (path == NULL)) {
- virHookReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to build path for %s hook"),
- drvstr);
- return -1;
- }
+ virBuildPath(path, LIBVIRT_HOOK_DIR, drvstr);
cmd = virCommandNew(path);
VIR_FREE(path);
diff --git a/src/util/virfcp.c b/src/util/virfcp.c
index 80773c7c5d..5e8fe72fec 100644
--- a/src/util/virfcp.c
+++ b/src/util/virfcp.c
@@ -40,8 +40,7 @@ virFCIsCapableRport(const char *rport)
{
g_autofree char *path = NULL;
-if (virBuildPath(, SYSFS_FC_RPORT_PATH, rport) < 0)
-return false;
+virBuildPath(, SYSFS_FC_RPORT_PATH, rport);
return virFileExists(path);
}
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 5710495bbf..27a647d1d0 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1308,13 +1308,12 @@ virFileFindMountPoint(const char *type G_GNUC_UNUSED)
#endif /* defined WITH_MNTENT_H && defined WITH_GETMNTENT_R */
-int
+void
virBuildPathInternal(char **path, ...)
{
char *path_component = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
va_list ap;
-int ret = 0;
va_start(ap, path);
@@ -1329,10 +1328,6 @@ virBuildPathInternal(char **path, ...)
va_end(ap);
*path = virBufferContentAndReset();
-if (*path == NULL)
-ret = -1;
-
-return ret;
}
/* Read no more than the specified maximum number of bytes. */
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 733d652ac9..f237e98290 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -300,7 +300,7 @@ char *virFileFindMountPoint(const char *type);
/* NB: this should be combined with virFileBuildPath */
#define virBuildPath(path, ...) \
virBuildPathInternal(path, __VA_ARGS__, NULL)
-int virBuildPathInternal(char **path, ...) G_GNUC_NULL_TERMINATED;
+void virBuildPathInternal(char **path, ...) G_GNUC_NULL_TERMINATED;
typedef struct _virHugeTLBFS virHugeTLBFS;
typedef virHugeTLBFS *virHugeTLBFSPtr;
diff --git a/src/util/virhook.c b/src/util/virhook.c
index e4e1945225..b52e2cd814 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -155,12 +155,7 @@ virHookCheck(int no, const char *driver)
return -1;
}
-if (virBuildPath(, LIBVIRT_HOOK_DIR, driver) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to build path for %s hook"),
- driver);
-return -1;
-}
+virBuildPath(, LIBVIRT_HOOK_DIR, driver);
if (!virFileExists(path)) {
VIR_DEBUG("No hook script %s", path);
@@ -405,12 +400,7 @@ virHookCall(int driver,
if (extra == NULL)
extra = "-";
-if (virBuildPath(, LIBVIRT_HOOK_DIR, drvstr) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to build path for %s hook"),
- drvstr);
-return -1;
-}
+virBuildPath(, LIBVIRT_HOOK_DIR, drvstr);
script_ret = 1;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 3df4199532..515b642db4 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2293,10 +2293,7 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path,
*pf = NULL;
-if (virBuildPath(_link, vf_sysfs_path, "physfn") == -1) {
-virReportOOMError();
-return -1;
-}
+virBuildPath(_link, vf_sysfs_path, "physfn");
if ((*pf = virPCIGetDeviceAddressFromSysfsLink(device_link))) {
VIR_DEBUG("PF for VF device '%s': " VIR_PCI_DEVICE_ADDRESS_FMT,
@@ -2470,11 +2467,7 @@ virPCIGetNetName(const char *device_link_sysfs_path,
*netname = NULL;
-if (virBuildPath(_sysfs_net_path, device_link_sysfs_path,
- "net") == -1) {
-virReportOOMError();
-return -1;
-}
+virBuildPath(_sysfs_net_path, device_link_sysfs_path, "net");
if (virDirOpenQuiet(, pcidev_sysfs_net_path) < 0) {
/* this *isn't* an error - caller needs to check for netname == NULL */
--
2.29.2
[PATCH 07/33] virhostcputest: linuxCPUStatsCompareFiles: Don't check return value of virBufferContentAndReset
The buffer won't encounter OOM condition nowadays
Signed-off-by: Peter Krempa
---
tests/virhostcputest.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tests/virhostcputest.c b/tests/virhostcputest.c
index 786a363e48..2608f0becc 100644
--- a/tests/virhostcputest.c
+++ b/tests/virhostcputest.c
@@ -141,10 +141,7 @@ linuxCPUStatsCompareFiles(const char *cpustatfile,
goto fail;
}
-if (!(actualData = virBufferContentAndReset())) {
-virReportOOMError();
-goto fail;
-}
+actualData = virBufferContentAndReset();
if (virTestCompareToFile(actualData, outfile) < 0)
goto fail;
--
2.29.2
[PATCH 04/33] virCommandAddEnv: Make stealing of argument more obvious
The function is supposed to always consume the passed environment
variable string. Use a temp variable with autofree and g_steal_pointer
to prevent having to free it manually.
Signed-off-by: Peter Krempa
---
src/util/vircommand.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 323f841b98..b94ab615d5 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1325,8 +1325,10 @@ virCommandRawStatus(virCommandPtr cmd)
* already set, then it is replaced in the list.
*/
static void
-virCommandAddEnv(virCommandPtr cmd, char *env)
+virCommandAddEnv(virCommandPtr cmd,
+ char *envstr)
{
+g_autofree char *env = envstr;
size_t namelen;
size_t i;
@@ -1336,19 +1338,18 @@ virCommandAddEnv(virCommandPtr cmd, char *env)
/* + 1 because we want to match the '=' character too. */
if (STREQLEN(cmd->env[i], env, namelen + 1)) {
VIR_FREE(cmd->env[i]);
-cmd->env[i] = env;
+cmd->env[i] = g_steal_pointer();
return;
}
}
/* Arg plus trailing NULL. */
if (VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 1 + 1) < 0) {
-VIR_FREE(env);
cmd->has_error = ENOMEM;
return;
}
-cmd->env[cmd->nenv++] = env;
+cmd->env[cmd->nenv++] = g_steal_pointer();
}
/**
--
2.29.2
[PATCH 03/33] virDomainDefSetMetadata: Avoid temporary variable for string copy
Since error checking was removed when switching to g_strdup, it doesn't
make much sense to have 'tmp' around.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b731744f04..6f4487fcfc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30426,7 +30426,6 @@ virDomainDefSetMetadata(virDomainDefPtr def,
xmlDocPtr doc = NULL;
xmlNodePtr old;
xmlNodePtr new = NULL;
-char *tmp = NULL;
int ret = -1;
if (type >= VIR_DOMAIN_METADATA_LAST) {
@@ -30437,19 +30436,17 @@ virDomainDefSetMetadata(virDomainDefPtr def,
switch ((virDomainMetadataType) type) {
case VIR_DOMAIN_METADATA_DESCRIPTION:
-if (STRNEQ_NULLABLE(metadata, ""))
-tmp = g_strdup(metadata);
+g_clear_pointer(>description, g_free);
-VIR_FREE(def->description);
-def->description = tmp;
+if (STRNEQ_NULLABLE(metadata, ""))
+def->description = g_strdup(metadata);
break;
case VIR_DOMAIN_METADATA_TITLE:
-if (STRNEQ_NULLABLE(metadata, ""))
-tmp = g_strdup(metadata);
+g_clear_pointer(>title, g_free);
-VIR_FREE(def->title);
-def->title = tmp;
+if (STRNEQ_NULLABLE(metadata, ""))
+def->title = g_strdup(metadata);
break;
case VIR_DOMAIN_METADATA_ELEMENT:
--
2.29.2
[PATCH 02/33] util: xml: Introduce autoptr cleanup support for 'xmlNode'
Signed-off-by: Peter Krempa --- src/util/virxml.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virxml.h b/src/util/virxml.h index f73b8975ba..e696dd25f5 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -255,6 +255,7 @@ G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virXPathContextNodeSave, virXPathContextNodeRes G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlDoc, xmlFreeDoc); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlXPathContext, xmlXPathFreeContext); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlBuffer, xmlBufferFree); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlNode, xmlFreeNode); typedef int (*virXMLNamespaceParse)(xmlXPathContextPtr ctxt, void **nsdata); typedef void (*virXMLNamespaceFree)(void *nsdata); -- 2.29.2
[PATCH 01/33] Remove useless comments for VIR_FROM_THIS definition
Signed-off-by: Peter Krempa
---
src/util/virpci.c | 1 -
src/util/virscsi.c | 1 -
src/util/virscsivhost.c | 1 -
3 files changed, 3 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 8147ce11e9..3df4199532 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -103,7 +103,6 @@ struct _virPCIDeviceList {
};
-/* For virReportOOMError() and virReportSystemError() */
#define VIR_FROM_THIS VIR_FROM_NONE
/* Specifications referenced in comments:
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 2a9f6da76a..d0ba33e254 100644
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -37,7 +37,6 @@
#define SYSFS_SCSI_DEVICES "/sys/bus/scsi/devices"
-/* For virReportOOMError() and virReportSystemError() */
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("util.scsi");
diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
index b7bf0da1b3..a0dfb8021a 100644
--- a/src/util/virscsivhost.c
+++ b/src/util/virscsivhost.c
@@ -28,7 +28,6 @@
#include "virstring.h"
#include "viralloc.h"
-/* For virReportOOMError() and virReportSystemError() */
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("util.scsihost");
--
2.29.2
[PATCH 00/33] Remove the now misleading virReportOOMError infra
'virReportOOMError' is nowadays mostly useless since an OOM error will trigger yet another allocation failure in the logger when attempting to log the error. Additionally it was also used in few places which can fail with other failures than OOM. To prevent both errorneous usage types, let's remove it completely. In the rare case we'd want to allocate a massive buffer and want to fail gracefully we still can use virReportError with VIR_ERR_NO_MEMORY. The series starts by some cleanup, then removes error handling from places where it's now dead code as we'll abort before the reporting, then converts some real OOM scenarios to abort() directly and lastly fixes the error message for cases where other errors are possible and lastly removes virReportOOMError. Pipelines are stuck, so hopefully it will go through: https://gitlab.com/pipo.sk/libvirt/-/pipelines/261161989 Peter Krempa (33): Remove useless comments for VIR_FROM_THIS definition util: xml: Introduce autoptr cleanup support for 'xmlNode' virDomainDefSetMetadata: Avoid temporary variable for string copy virCommandAddEnv: Make stealing of argument more obvious virCommandAddArgBuffer: Simplify clearing of @buf virCPUx86DataParse: Don't check error from x86FeatureNames virhostcputest: linuxCPUStatsCompareFiles: Don't check return value of virBufferContentAndReset virBuildPath: Remove return value lxc_process: Remove OOM handling from logging setup virDomainDefSetMetadata: Rework memory handling util: vircommand: Remove OOM handling virCloseCallbacksGetForConn: Remove OOM handling virfirewall: Don't check OOM in ADD_ARG macro virfirewall: Remove OOM checks from virFirewallStartTransaction virfirewall: virFirewallAddRuleFullV: Remove OOM check from VIR_APPEND_ELEMENT virfirewall: Remove impossible OOM error reporting util: virnetlink: Add wrapper for 'nlmsg_alloc_simple' util: xml: Add virXMLBufferCreate wrapper util: xml: Add wrapper for 'xmlNewNode' Don't report OOM error on xmlCopyNode failure virXMLXPathContextNew: abort() on allocation failure virXMLParseHelper: abort() on allocation failure util: virprocess: abort() on CPU_ALLOC failure virURIFormat: abort() on failure util: iohelper: Don't handle OOM from posix_memalign hyperv: abort() failure of wsmc_fault_new() vbox: abort() on allocation failure in UTF8<->UTF16 conversion libxl: abort() on failure of libxl_cpu_bitmap_alloc() virVBoxSnapshotConfSaveVboxFile: abort() on failure to allocate xmlDoc and comment util: json: Report non-OOM error on yajl failure storage: Don't report OOM error on failure of glfs_new virVMXConvertToUTF8: Report non-OOM error on failure of xmlBufferCreateStatic util: virerror: Remove virReportOOMError build-aux/syntax-check.mk | 8 -- docs/internals/command.html.in| 8 +- src/conf/domain_conf.c| 62 - src/conf/network_conf.c | 5 +- src/cpu/cpu_x86.c | 6 +- src/hyperv/hyperv_wmi.c | 8 +- src/hypervisor/virclosecallbacks.c| 15 +-- src/libvirt_private.syms | 3 +- src/libxl/libxl_conf.c| 6 +- src/libxl/libxl_driver.c | 7 +- src/lxc/lxc_process.c | 9 +- src/storage/storage_backend_gluster.c | 3 +- .../storage_file_backend_gluster.c| 3 +- src/test/test_driver.c| 6 +- src/util/iohelper.c | 7 +- src/util/vircommand.c | 119 +- src/util/virerror.c | 22 src/util/virerror.h | 8 -- src/util/virfcp.c | 3 +- src/util/virfile.c| 7 +- src/util/virfile.h| 2 +- src/util/virfirewall.c| 62 ++--- src/util/virhook.c| 14 +-- src/util/virjson.c| 6 +- src/util/virnetdev.c | 18 +-- src/util/virnetdevbridge.c| 10 +- src/util/virnetdevip.c| 15 +-- src/util/virnetdevvportprofile.c | 6 +- src/util/virnetlink.c | 38 +++--- src/util/virnetlink.h | 4 + src/util/virpci.c | 12 +- src/util/virprocess.c | 12 +- src/util/virscsi.c| 1 - src/util/virscsivhost.c | 1 - src/util/viruri.c | 8 +- src/util/virxml.c | 44 --- src/util/virxml.h | 8 ++ src/vbox/vbox_common.c| 20 --- src/vbox/vbox_common.h| 15 ++-
Re: [PATCH v2 00/31] qapi/qom: QAPIfy --object and object-add
On Wed, Feb 24, 2021 at 14:52:24 +0100, Kevin Wolf wrote: > This series adds a QAPI type for the properties of all user creatable > QOM types and finally makes the --object command line option (in all > binaries) and the object-add monitor commands (in QMP and HMP) use the > new ObjectOptions union. > > This change improves things in more than just one way: > > 1. Documentation for QOM object types has always been lacking. Adding >the schema, we get documentation for every property. > > 2. It prevents bugs by performing parts of the input validation (e.g. >checking presence of mandatory properties) already in QAPI instead of >relying on separate manual implementations in each class. > > 3. It provides QAPI introspection for user creatable objects. > > 4. Non-scalar properties are now supported everywhere because the >command line parsers (including HMP) use the keyval parser now. I've updated and posted another version of the libvirt patches which add testing that our generated props conform to the schema and also deals with the dropped 'props' wrapper: https://listman.redhat.com/archives/libvir-list/2021-February/msg01212.html Libvirt's test pass after it without any change, so on behalf of libvirt ACKed-by: Peter Krempa
[PATCH v2 12/12] [DON'T PUSH] Force-check all configs with latest capabilities
Hack the qemuxml2argvtest to force-validate everything with latest
capabilities. The result is expected:
190) QEMU XML-2-ARGV disk-network-tlsx509-vxhs.x86_64-5.0.0...
failed to validate -blockdev
'{"driver":"vxhs","tls-creds":"objlibvirt-3-storage_tls0","vdisk-id":"eb90327c-8302-4725-9e1b-4e85ed4dc251","server":{"host":"192.168.0.1","port":""},"node-name":"libvirt-3-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
against QAPI schema: {
ERROR: variant 'vxhs' for discriminator 'driver' not found
FAILED
1036) QEMU XML-2-ARGV launch-security-sev.x86_64-2.12.0 ...
failed to validate -object
'{"qom-type":"sev-guest","id":"sev0","cbitpos":47,"reduced-phys-bits":1,"policy":1,"dh-cert-file":"/tmp/lib/domain--1-QEMUGuest1/dh_cert.base64","session-file":"/tmp/lib/domain--1-QEMUGuest1/session.base64"}'
against QAPI schema: {
ERROR: variant 'sev-guest' for discriminator 'qom-type' not found
FAILED
1037) QEMU XML-2-ARGV launch-security-sev-missing-platform-info.x86_64-2.12.0
... failed to validate -object
'{"qom-type":"sev-guest","id":"sev0","cbitpos":47,"reduced-phys-bits":1,"policy":1,"dh-cert-file":"/tmp/lib/domain--1-QEMUGuest1/dh_cert.base64","session-file":"/tmp/lib/domain--1-QEMUGuest1/session.base64"}'
against QAPI schema: {
ERROR: variant 'sev-guest' for discriminator 'qom-type' not found
FAILED
'vxhs' was deprecated/removed and is also not an '-object' and
'sev-guest' was not enabled on the box I used for generating the
capabilities.
---
tests/qemuxml2argvtest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a5e40c218a..1c4ba16bbf 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -529,8 +529,8 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv,
bool netdevQAPIfied = false;
bool objectQAPIfied = false;
-/* comment out with line comment to enable schema checking for non _CAPS
tests
-if (!info->schemafile)
+///* comment out with line comment to enable schema checking for non _CAPS
tests
+//if (!info->schemafile)
info->schemafile =
testQemuGetLatestCapsForArch(virArchToString(info->arch), "replies");
// */
--
2.29.2
[PATCH v2 03/12] qemu: command: Generate commandline of iothread objects JSON
The commandline generator for 'iothread' objects has a private
implementation of the properties. Convert it to JSON so that it can be
later validated.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 579b00c029..7bdcdab95a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7228,15 +7228,19 @@ qemuBuildIOThreadCommandLine(virCommandPtr cmd,
if (def->niothreadids == 0)
return 0;
-/* Create iothread objects using the defined iothreadids list
- * and the defined id and name from the list. These may be used
- * by a disk definition which will associate to an iothread by
- * supplying a value of an id from the list
- */
for (i = 0; i < def->niothreadids; i++) {
+g_autoptr(virJSONValue) props = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+g_autofree char *alias = g_strdup_printf("iothread%u",
def->iothreadids[i]->iothread_id);
+
+if (qemuMonitorCreateObjectProps(, "iothread", alias, NULL) < 0)
+return -1;
+
+if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0)
+return -1;
+
virCommandAddArg(cmd, "-object");
-virCommandAddArgFormat(cmd, "iothread,id=iothread%u",
- def->iothreadids[i]->iothread_id);
+virCommandAddArgBuffer(cmd, );
}
return 0;
--
2.29.2
[PATCH v2 04/12] qemu: capabilities: Introduce QEMU_CAPS_OBJECT_QAPIFIED
Starting from qemu-6.0 the parameters of -object/object-add are formally
described by the QAPI schema. Additionally this changes the nesting of
the properties as the 'props' nested object will be flattened to the
parent.
We'll need to detect whether qemu switched to this new approach to
generate the objects with proper nesting and also allow testing.
The capability is based on the presence of the 'secret' object in the
'qom-type' enum.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_capabilities.c | 4
src/qemu/qemu_capabilities.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f40d6d77be..d1452f6354 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -618,6 +618,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
"memory-backend-file.x-use-canonical-path-for-ramblock-id",
"vnc-opts",
"migration-param.block-bitmap-mapping",
+
+ /* 395 */
+ "object.qapified",
);
@@ -1553,6 +1556,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsQMPSchemaQueries[] = {
{ "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA },
{ "migrate-set-parameters/arg-type/block-bitmap-mapping/bitmaps/transform",
QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING },
+{ "object-add/arg-type/qom-type/^secret", QEMU_CAPS_OBJECT_QAPIFIED },
};
typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index a5b6c7f104..193432246d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -599,6 +599,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_VNC_OPTS, /* -vnc uses QemuOpts parser instead of custom code */
QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING, /* block-bitmap-mapping in
migrate-set-parameters */
+/* 395 */
+QEMU_CAPS_OBJECT_QAPIFIED, /* parameters for object-add are formally
described */
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.29.2
[PATCH v2 02/12] qemu: command: Generate commandline of 'sev0' sev-guest object via JSON
While the 'sev0' sev-guest object will never be hotplugged, but we want
to generate it through JSON so that we'll be able to validate all
parameters of '-object' against the QAPI schema once 'object-add' is
qapified in qemu.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 32 +++
...v-missing-platform-info.x86_64-2.12.0.args | 2 +-
.../launch-security-sev.x86_64-2.12.0.args| 2 +-
3 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9538bc9a2a..579b00c029 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9444,9 +9444,11 @@ static int
qemuBuildSEVCommandLine(virDomainObjPtr vm, virCommandPtr cmd,
virDomainSEVDefPtr sev)
{
+g_autoptr(virJSONValue) props = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
qemuDomainObjPrivatePtr priv = vm->privateData;
-char *path = NULL;
+g_autofree char *dhpath = NULL;
+g_autofree char *sessionpath = NULL;
if (!sev)
return 0;
@@ -9454,21 +9456,23 @@ qemuBuildSEVCommandLine(virDomainObjPtr vm,
virCommandPtr cmd,
VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
sev->policy, sev->cbitpos, sev->reduced_phys_bits);
-virBufferAsprintf(, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos);
-virBufferAsprintf(, ",reduced-phys-bits=%d", sev->reduced_phys_bits);
-virBufferAsprintf(, ",policy=0x%x", sev->policy);
+if (sev->dh_cert)
+dhpath = g_strdup_printf("%s/dh_cert.base64", priv->libDir);
-if (sev->dh_cert) {
-path = g_strdup_printf("%s/dh_cert.base64", priv->libDir);
-virBufferAsprintf(, ",dh-cert-file=%s", path);
-VIR_FREE(path);
-}
+if (sev->session)
+sessionpath = g_strdup_printf("%s/session.base64", priv->libDir);
-if (sev->session) {
-path = g_strdup_printf("%s/session.base64", priv->libDir);
-virBufferAsprintf(, ",session-file=%s", path);
-VIR_FREE(path);
-}
+if (qemuMonitorCreateObjectProps(, "sev-guest", "sev0",
+ "u:cbitpos", sev->cbitpos,
+ "u:reduced-phys-bits",
sev->reduced_phys_bits,
+ "u:policy", sev->policy,
+ "S:dh-cert-file", dhpath,
+ "S:session-file", sessionpath,
+ NULL) < 0)
+return -1;
+
+if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0)
+return -1;
virCommandAddArg(cmd, "-object");
virCommandAddArgBuffer(cmd, );
diff --git
a/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args
b/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args
index f6cbd016df..717a21b7b0 100644
---
a/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args
+++
b/tests/qemuxml2argvdata/launch-security-sev-missing-platform-info.x86_64-2.12.0.args
@@ -29,7 +29,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x1,\
+-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=1,\
dh-cert-file=/tmp/lib/domain--1-QEMUGuest1/dh_cert.base64,\
session-file=/tmp/lib/domain--1-QEMUGuest1/session.base64 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
diff --git a/tests/qemuxml2argvdata/launch-security-sev.x86_64-2.12.0.args
b/tests/qemuxml2argvdata/launch-security-sev.x86_64-2.12.0.args
index f6cbd016df..717a21b7b0 100644
--- a/tests/qemuxml2argvdata/launch-security-sev.x86_64-2.12.0.args
+++ b/tests/qemuxml2argvdata/launch-security-sev.x86_64-2.12.0.args
@@ -29,7 +29,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x1,\
+-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=1,\
dh-cert-file=/tmp/lib/domain--1-QEMUGuest1/dh_cert.base64,\
session-file=/tmp/lib/domain--1-QEMUGuest1/session.base64 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
--
2.29.2
[PATCH v2 06/12] qemu: command: Introduce raw JSON passthrough for '-object' for testing
The qemu commandline builder's QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON
flag disables JSON->commandline conversion so that our qemuxml2argvtest
can use the commandline test repostitory for validating our JSON props
generators which are in many cases used on the montitor where we need to
conform to the schema.
Wire this up for the -object/object-add pair by adding a flag to
virQEMUBuildObjectCommandlineFromJSON to pass through JSON and pipe
through the 'flags' variable where necessary.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 264
src/util/virqemu.c | 16 ++-
src/util/virqemu.h | 3 +-
3 files changed, 176 insertions(+), 107 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7bdcdab95a..c4bb6a87bb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -190,7 +190,8 @@ VIR_ENUM_IMPL(qemuNumaPolicy,
*/
static int
qemuBuildMasterKeyCommandLine(virCommandPtr cmd,
- qemuDomainObjPrivatePtr priv)
+ qemuDomainObjPrivatePtr priv,
+ unsigned int flags)
{
g_autofree char *alias = NULL;
g_autofree char *path = NULL;
@@ -223,7 +224,8 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd,
NULL) < 0)
return -1;
-if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0)
+if (virQEMUBuildObjectCommandlineFromJSON(, props,
+ (flags &
QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON)) < 0)
return -1;
virCommandAddArg(cmd, "-object");
@@ -699,6 +701,7 @@ qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
* qemuBuildObjectSecretCommandLine:
* @cmd: the command to modify
* @secinfo: pointer to the secret info object
+ * @flags: commandline builder flags
*
* If the secinfo is available and associated with an AES secret,
* then format the command line for the secret object. This object
@@ -709,7 +712,8 @@ qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
*/
static int
qemuBuildObjectSecretCommandLine(virCommandPtr cmd,
- qemuDomainSecretInfoPtr secinfo)
+ qemuDomainSecretInfoPtr secinfo,
+ unsigned int flags)
{
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
g_autoptr(virJSONValue) props = NULL;
@@ -717,7 +721,8 @@ qemuBuildObjectSecretCommandLine(virCommandPtr cmd,
if (qemuBuildSecretInfoProps(secinfo, ) < 0)
return -1;
-if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0)
+if (virQEMUBuildObjectCommandlineFromJSON(, props,
+ (flags &
QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON)) < 0)
return -1;
virCommandAddArg(cmd, "-object");
@@ -865,6 +870,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
* (optional)
* @alias: TLS object alias
* @qemuCaps: capabilities
+ * @flags: commandline builder flags
*
* Create the command line for a TLS object
*
@@ -877,7 +883,8 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
bool verifypeer,
const char *certEncSecretAlias,
const char *alias,
-virQEMUCapsPtr qemuCaps)
+virQEMUCapsPtr qemuCaps,
+unsigned int flags)
{
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
g_autoptr(virJSONValue) props = NULL;
@@ -886,7 +893,8 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
certEncSecretAlias, qemuCaps, ) < 0)
return -1;
-if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0)
+if (virQEMUBuildObjectCommandlineFromJSON(, props,
+ (flags &
QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON)) < 0)
return -1;
virCommandAddArg(cmd, "-object");
@@ -1976,14 +1984,16 @@
qemuBuildFloppyCommandLineControllerOptions(virCommandPtr cmd,
static int
qemuBuildObjectCommandline(virCommandPtr cmd,
- virJSONValuePtr objProps)
+ virJSONValuePtr objProps,
+ unsigned int flags)
{
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
if (!objProps)
return 0;
-if (virQEMUBuildObjectCommandlineFromJSON(, objProps) < 0)
+if (virQEMUBuildObjectCommandlineFromJSON(, objProps,
+ (flags &
QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON)) < 0)
return -1;
virCommandAddArg(cmd, "-object");
@@ -1995,16 +2005,17 @@ qemuBuildObjectCommandline(virCommandPtr cmd,
static int
qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd,
-
[PATCH v2 10/12] qemumonitorjsontest: Remove bomb guarding object-add
Libvirt is now prepared for QAPIfied object-add.
Signed-off-by: Peter Krempa
---
tests/qemumonitorjsontest.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 82c74e2ef9..48b41c908a 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2740,20 +2740,6 @@ testQAPISchemaObjectDeviceAdd(const void *opaque)
return -1;
}
-if (virQEMUQAPISchemaPathGet("object-add/arg-type", schema, ) < 0) {
-fprintf(stderr, "schema for 'objectadd' not found\n");
-return -1;
-}
-
-if (testQEMUSchemaEntryMatchTemplate(entry,
- "str:qom-type",
- "str:id",
- "any:props",
- NULL) < 0) {
-VIR_TEST_VERBOSE("object-add has unexpected members in schema");
-return -1;
-}
-
return 0;
}
--
2.29.2
[PATCH v2 01/12] qemu: command: Generate commandline of 'masterKey0' secret via JSON
While the 'masterKey0' secret object will never be hotplugged we want to generate it through JSON so that we'll be able to validate all parameters of '-object' against the QAPI schema once 'object-add' is qapified in qemu. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f255b0f881..9538bc9a2a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -195,6 +195,7 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd, g_autofree char *alias = NULL; g_autofree char *path = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; +g_autoptr(virJSONValue) props = NULL; /* If the -object secret does not exist, then just return. This just * means the domain won't be able to use a secret master key and is @@ -216,9 +217,16 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd, if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir))) return -1; +if (qemuMonitorCreateObjectProps(, "secret", alias, + "s:format", "raw", + "s:file", path, + NULL) < 0) +return -1; + +if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0) +return -1; + virCommandAddArg(cmd, "-object"); -virBufferAsprintf(, "secret,id=%s,format=raw,file=", alias); -virQEMUBuildBufferEscapeComma(, path); virCommandAddArgBuffer(cmd, ); return 0; -- 2.29.2
[PATCH v2 08/12] qemuMonitorCreateObjectPropsWrap: Open-code in qemuBuildMemoryBackendProps
There's just one caller left. Since qemuBuildMemoryBackendProps is too
complex to be modified for now, just move the adding of 'id' and 'qom'
type directly into the function.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 6 --
src/qemu/qemu_monitor.c | 15 ---
src/qemu/qemu_monitor.h | 4
3 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c4bb6a87bb..31b20a4f12 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3284,10 +3284,12 @@ qemuBuildMemoryBackendProps(virJSONValuePtr
*backendProps,
rc = 0;
}
-if (!(*backendProps = qemuMonitorCreateObjectPropsWrap(backendType, alias,
- )))
+if (virJSONValueObjectPrependString(props, "id", alias) < 0 ||
+virJSONValueObjectPrependString(props, "qom-type", backendType) < 0)
return -1;
+*backendProps = g_steal_pointer();
+
return rc;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 8b88c8f922..121c21be5c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3010,21 +3010,6 @@ qemuMonitorAddDeviceArgs(qemuMonitorPtr mon,
}
-virJSONValuePtr
-qemuMonitorCreateObjectPropsWrap(const char *type,
- const char *alias,
- virJSONValuePtr *props)
-{
-
-if (virJSONValueObjectPrependString(*props, "id", alias) < 0 ||
-virJSONValueObjectPrependString(*props, "qom-type", type))
-return NULL;
-
-return g_steal_pointer(props);
-}
-
-
-
/**
* qemuMonitorCreateObjectProps:
* @propsret: returns full object properties
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d25c26343a..c6ffd51ce8 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1006,10 +1006,6 @@ int qemuMonitorAddDeviceWithFd(qemuMonitorPtr mon,
int qemuMonitorDelDevice(qemuMonitorPtr mon,
const char *devalias);
-virJSONValuePtr qemuMonitorCreateObjectPropsWrap(const char *type,
- const char *alias,
- virJSONValuePtr *props);
-
int qemuMonitorCreateObjectProps(virJSONValuePtr *propsret,
const char *type,
const char *alias,
--
2.29.2
[PATCH v2 11/12] tests: qemucapabilities: Update qemu caps for object-add qapification
qemu qapified object-add, which means that it's introspectable via
query-qmp-schema. Update the qemu-6.0 capabilities to commit
XX
TODO: update to pushed version
---
.../caps_6.0.0.x86_64.replies | 3238 -
.../caps_6.0.0.x86_64.xml | 83 +-
2 files changed, 2471 insertions(+), 850 deletions(-)
diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies
b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies
index 04ebd04583..d878522c75 100644
--- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies
@@ -21,7 +21,7 @@
"minor": 2,
"major": 5
},
-"package": "v5.2.0-2208-gc79f01c945"
+"package": "v5.2.0-2362-g25fa357a8e"
},
"id": "libvirt-2"
}
@@ -45,9 +45,6 @@
{
"return": [
-{
- "name": "object-add"
-},
{
"name": "device_add"
},
@@ -210,6 +207,9 @@
{
"name": "object-del"
},
+{
+ "name": "object-add"
+},
{
"name": "qom-list-properties"
},
@@ -667,6 +667,10 @@
"name": "vhost-user-vsock-device",
"parent": "vhost-vsock-common"
},
+{
+ "name": "virtio-blk-pci-transitional",
+ "parent": "virtio-blk-pci-base"
+},
{
"name": "pcie-pci-bridge",
"parent": "base-pci-bridge"
@@ -675,6 +679,10 @@
"name": "pc-q35-2.11-machine",
"parent": "generic-pc-machine"
},
+{
+ "name": "virtio-crypto-device",
+ "parent": "virtio-device"
+},
{
"name": "isa-applesmc",
"parent": "isa-device"
@@ -692,35 +700,35 @@
"parent": "bus"
},
{
- "name": "virtio-crypto-device",
- "parent": "virtio-device"
+ "name": "Denverton-x86_64-cpu",
+ "parent": "x86_64-cpu"
},
{
"name": "chardev-testdev",
"parent": "chardev"
},
{
- "name": "Denverton-x86_64-cpu",
- "parent": "x86_64-cpu"
+ "name": "usb-wacom-tablet",
+ "parent": "usb-device"
},
{
- "name": "pci-ipmi-bt",
- "parent": "pci-device"
+ "name": "Icelake-Server-v1-x86_64-cpu",
+ "parent": "x86_64-cpu"
},
{
- "name": "sev-guest",
- "parent": "confidential-guest-support"
+ "name": "chardev-stdio",
+ "parent": "chardev-fd"
},
{
- "name": "usb-redir",
- "parent": "usb-device"
+ "name": "pci-ipmi-bt",
+ "parent": "pci-device"
},
{
"name": "filter-buffer",
"parent": "netfilter"
},
{
- "name": "usb-wacom-tablet",
+ "name": "usb-redir",
"parent": "usb-device"
},
{
@@ -732,8 +740,8 @@
"parent": "pci-vga"
},
{
- "name": "virtio-blk-pci-transitional",
- "parent": "virtio-blk-pci-base"
+ "name": "kvm-pit",
+ "parent": "pit-common"
},
{
"name": "Haswell-v1-x86_64-cpu",
@@ -756,8 +764,8 @@
"parent": "generic-pc-machine"
},
{
- "name": "kvm-pit",
- "parent": "pit-common"
+ "name": "sev-guest",
+ "parent": "confidential-guest-support"
},
{
"name": "ich9-usb-uhci5",
@@ -812,8 +820,8 @@
"parent": "usb-device"
},
{
- "name": "chardev-serial",
- "parent": "chardev-fd"
+ "name": "chardev-pty",
+ "parent": "chardev"
},
{
"name": "virtio-blk-device",
@@ -848,8 +856,8 @@
"parent": "generic-pc-machine"
},
{
- "name": "chardev-pty",
- "parent": "chardev"
+ "name": "chardev-serial",
+ "parent": "chardev-fd"
},
{
"name": "qtest-accel",
@@ -979,14 +987,14 @@
"name": "pci-ipmi-kcs",
"parent": "pci-device"
},
-{
- "name": "intel-iommu-iommu-memory-region",
- "parent": "qemu:iommu-memory-region"
-},
{
"name": "xio3130-downstream",
"parent": "pcie-slot"
},
+{
+ "name": "intel-iommu-iommu-memory-region",
+ "parent": "qemu:iommu-memory-region"
+},
{
"name": "vhost-user-vsock-pci-non-transitional",
"parent": "vhost-user-vsock-pci-base"
@@ -995,14 +1003,14 @@
"name": "pc-i440fx-2.3-machine",
"parent": "generic-pc-machine"
},
-{
- "name": "PCI",
- "parent": "bus"
-},
{
"name": "microvm-machine",
"parent": "x86-machine"
},
+{
+ "name": "PCI",
+ "parent": "bus"
+},
{
"name": "sdhci-bus",
"parent": "sd-bus"
@@ -1104,8 +1112,8 @@
"parent": "pci-device"
},
{
- "name": "virtio-input-host-pci",
- "parent": "virtio-input-host-pci-base-type"
+ "name": "virtio-9p-pci-transitional",
+ "parent": "virtio-9p-pci-base"
},
{
"name": "nvdimm",
@@ -1116,8 +1124,8 @@
"parent": "generic-pc-machine"
},
{
- "name": "virtio-9p-pci-transitional",
- "parent": "virtio-9p-pci-base"
+
[PATCH v2 05/12] tests: qemuxml2argv: Validate generation of JSON props for object-add
Similarly to the validation for blockdev-add and netdev_add, use the
qemuxml2argv test repository to drive validation of props for
object-add.
Signed-off-by: Peter Krempa
---
tests/qemuxml2argvtest.c | 20
1 file changed, 20 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d6d707cd24..a5e40c218a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -527,6 +527,7 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv,
g_autoptr(virCommand) cmd = NULL;
unsigned int parseFlags = info->parseFlags;
bool netdevQAPIfied = false;
+bool objectQAPIfied = false;
/* comment out with line comment to enable schema checking for non _CAPS
tests
if (!info->schemafile)
@@ -570,6 +571,7 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv,
return -1;
netdevQAPIfied =
!virQEMUQAPISchemaPathExists("netdev_add/arg-type/type/!string", schema);
+objectQAPIfied =
virQEMUQAPISchemaPathExists("object-add/arg-type/qom-type/^secret", schema);
for (i = 0; i < nargs; i++) {
g_auto(virBuffer) debug = VIR_BUFFER_INITIALIZER;
@@ -603,6 +605,24 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv,
return -1;
}
+i++;
+} else if (STREQ(args[i], "-object")) {
+
+if (!objectQAPIfied) {
+i++;
+continue;
+}
+
+if (!(jsonargs = virJSONValueFromString(args[i + 1])))
+return -1;
+
+if (testQEMUSchemaValidateCommand("object-add", jsonargs,
+ schema, false, false, ) <
0) {
+VIR_TEST_VERBOSE("failed to validate -object '%s' against QAPI
schema: %s",
+ args[i + 1], virBufferCurrentContent());
+return -1;
+}
+
i++;
}
}
--
2.29.2
[PATCH v2 07/12] qemu: monitor: Make wrapping of 'props' of 'object-add' optional
Construct the JSON object which is used for object-add without the
'props' wrapper and add the wrapper only in the monitor code.
This simplifies the JSON->commandline generator in the first place and
also prepares for upcoming qemu where 'props' will be removed.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 68 +
src/util/virqemu.c | 42 +
2 files changed, 62 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ed35da17e1..8b88c8f922 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -109,6 +109,9 @@ struct _qemuMonitor {
qemuMonitorReportDomainLogError logFunc;
void *logOpaque;
virFreeCallback logDestroy;
+
+/* true if qemu no longer wants 'props' sub-object of object-add */
+bool objectAddNoWrap;
};
/**
@@ -3012,14 +3015,12 @@ qemuMonitorCreateObjectPropsWrap(const char *type,
const char *alias,
virJSONValuePtr *props)
{
-virJSONValuePtr ret;
-ignore_value(virJSONValueObjectCreate(,
- "s:qom-type", type,
- "s:id", alias,
- "A:props", props,
- NULL));
-return ret;
+if (virJSONValueObjectPrependString(*props, "id", alias) < 0 ||
+virJSONValueObjectPrependString(*props, "qom-type", type))
+return NULL;
+
+return g_steal_pointer(props);
}
@@ -3039,26 +3040,28 @@ qemuMonitorCreateObjectProps(virJSONValuePtr *propsret,
const char *alias,
...)
{
-virJSONValuePtr props = NULL;
-int ret = -1;
+g_autoptr(virJSONValue) props = NULL;
+int rc;
va_list args;
-*propsret = NULL;
+if (virJSONValueObjectCreate(,
+ "s:qom-type", type,
+ "s:id", alias,
+ NULL) < 0)
+return -1;
+
va_start(args, alias);
-if (virJSONValueObjectCreateVArgs(, args) < 0)
-goto cleanup;
+rc = virJSONValueObjectAddVArgs(props, args);
-if (!(*propsret = qemuMonitorCreateObjectPropsWrap(type, alias, )))
-goto cleanup;
+va_end(args);
-ret = 0;
+if (rc < 0)
+return -1;
- cleanup:
-virJSONValueFree(props);
-va_end(args);
-return ret;
+*propsret = g_steal_pointer();
+return 0;
}
@@ -3078,6 +3081,7 @@ qemuMonitorAddObject(qemuMonitorPtr mon,
virJSONValuePtr *props,
char **alias)
{
+g_autoptr(virJSONValue) pr = NULL;
const char *type = NULL;
const char *id = NULL;
g_autofree char *aliasCopy = NULL;
@@ -3105,7 +3109,31 @@ qemuMonitorAddObject(qemuMonitorPtr mon,
if (alias)
aliasCopy = g_strdup(id);
-if (qemuMonitorJSONAddObject(mon, props) < 0)
+if (mon->objectAddNoWrap) {
+pr = g_steal_pointer(props);
+} else {
+/* we need to create a wrapper which has the 'qom-type' and 'id' and
+ * store everything else under a 'props' sub-object */
+g_autoptr(virJSONValue) typeobj = NULL;
+g_autoptr(virJSONValue) idobj = NULL;
+
+ignore_value(virJSONValueObjectRemoveKey(*props, "qom-type",
));
+ignore_value(virJSONValueObjectRemoveKey(*props, "id", ));
+
+if (!virJSONValueObjectGetKey(*props, 0)) {
+virJSONValueFree(*props);
+*props = NULL;
+}
+
+if (virJSONValueObjectCreate(,
+ "s:qom-type", type,
+ "s:id", id,
+ "A:props", props,
+ NULL) < 0)
+return -1;
+}
+
+if (qemuMonitorJSONAddObject(mon, ) < 0)
return -1;
if (alias)
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index c420b144e1..5fe142394c 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -303,32 +303,6 @@ virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr
props,
}
-static int
-virQEMUBuildObjectCommandlineFromJSONInternal(virBufferPtr buf,
- const char *type,
- const char *alias,
- virJSONValuePtr props)
-{
-if (!type || !alias) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("missing 'type'(%s) or 'alias'(%s) field of QOM
'object'"),
- NULLSTR(type), NULLSTR(alias));
-return -1;
-}
-
-virBufferAsprintf(buf, "%s,id=%s", type, alias);
-
-if (props) {
-virBufferAddLit(buf, ",");
-if (virQEMUBuildCommandLineJSON(props, buf, NULL,
-
[PATCH v2 00/12] qemu: Prepare for QAPIfied object-add
QEMU plans to QAPIfy object add. This series prepares for the API change (drop of 'props' wrapper for the object) and adds testing based on our qemuxml2argv test data which forces the output to JSON and validates it agains the schema. Based on Kevin's qemu patches: https://listman.redhat.com/archives/libvir-list/2021-February/msg01168.html Last patch forces more files to be processed and as the summary suggest will not be pushed. The patch updating qemucapabilitiesdata will be updated after Kevin's patches hit upstream repo. Peter Krempa (12): qemu: command: Generate commandline of 'masterKey0' secret via JSON qemu: command: Generate commandline of 'sev0' sev-guest object via JSON qemu: command: Generate commandline of iothread objects JSON qemu: capabilities: Introduce QEMU_CAPS_OBJECT_QAPIFIED tests: qemuxml2argv: Validate generation of JSON props for object-add qemu: command: Introduce raw JSON passthrough for '-object' for testing qemu: monitor: Make wrapping of 'props' of 'object-add' optional qemuMonitorCreateObjectPropsWrap: Open-code in qemuBuildMemoryBackendProps qemu: monitor: Don't add 'props' wrapper if qemu has QEMU_CAPS_OBJECT_QAPIFIED qemumonitorjsontest: Remove bomb guarding object-add tests: qemucapabilities: Update qemu caps for object-add qapification [DON'T PUSH] Force-check all configs with latest capabilities src/qemu/qemu_capabilities.c |4 + src/qemu/qemu_capabilities.h |3 + src/qemu/qemu_command.c | 326 +- src/qemu/qemu_monitor.c | 78 +- src/qemu/qemu_monitor.h |4 - src/util/virqemu.c| 48 +- src/util/virqemu.h|3 +- .../caps_6.0.0.x86_64.replies | 3238 - .../caps_6.0.0.x86_64.xml | 83 +- tests/qemumonitorjsontest.c | 14 - ...v-missing-platform-info.x86_64-2.12.0.args |2 +- .../launch-security-sev.x86_64-2.12.0.args|2 +- tests/qemuxml2argvtest.c | 24 +- 13 files changed, 2775 insertions(+), 1054 deletions(-) -- 2.29.2
[PATCH v2 09/12] qemu: monitor: Don't add 'props' wrapper if qemu has QEMU_CAPS_OBJECT_QAPIFIED
Set 'objectAddNoWrap' when the capability is present.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 121c21be5c..1e4c4809b1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -32,6 +32,7 @@
#include "qemu_monitor_json.h"
#include "qemu_domain.h"
#include "qemu_process.h"
+#include "qemu_capabilities.h"
#include "virerror.h"
#include "viralloc.h"
#include "virlog.h"
@@ -672,6 +673,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
qemuMonitorCallbacksPtr cb,
void *opaque)
{
+qemuDomainObjPrivatePtr priv = vm->privateData;
qemuMonitorPtr mon;
g_autoptr(GError) gerr = NULL;
@@ -704,6 +706,9 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
mon->cb = cb;
mon->callbackOpaque = opaque;
+if (priv)
+mon->objectAddNoWrap = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_OBJECT_QAPIFIED);
+
if (virSetCloseExec(mon->fd) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to set monitor close-on-exec flag"));
--
2.29.2
Re: [PATCH v2 28/31] hmp: QAPIfy object_add
* Kevin Wolf (kw...@redhat.com) wrote:
> This switches the HMP command object_add from a QemuOpts-based parser to
> user_creatable_add_from_str() which uses a keyval parser and enforces
> the QAPI schema.
>
> Apart from being a cleanup, this makes non-scalar properties and help
> accessible. In order for help to be printed to the monitor instead of
> stdout, the printf() calls in the help functions are changed to
> qemu_printf().
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Dr. David Alan Gilbert
> ---
> monitor/hmp-cmds.c | 17 ++---
> qom/object_interfaces.c | 11 ++-
> hmp-commands.hx | 2 +-
> 3 files changed, 9 insertions(+), 21 deletions(-)
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 3c88a4faef..652cf9ff21 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>
> void hmp_object_add(Monitor *mon, const QDict *qdict)
> {
> +const char *options = qdict_get_str(qdict, "object");
> Error *err = NULL;
> -QemuOpts *opts;
> -Object *obj = NULL;
> -
> -opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, );
> -if (err) {
> -goto end;
> -}
>
> -obj = user_creatable_add_opts(opts, );
> -qemu_opts_del(opts);
> -
> -end:
> +user_creatable_add_from_str(options, );
> hmp_handle_error(mon, err);
> -
> -if (obj) {
> -object_unref(obj);
> -}
> }
>
> void hmp_getfd(Monitor *mon, const QDict *qdict)
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 54f0dadfea..c4982dd7a0 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -13,6 +13,7 @@
> #include "qemu/help_option.h"
> #include "qemu/module.h"
> #include "qemu/option.h"
> +#include "qemu/qemu-print.h"
> #include "qapi/opts-visitor.h"
> #include "qemu/config-file.h"
>
> @@ -212,11 +213,11 @@ static void user_creatable_print_types(void)
> {
> GSList *l, *list;
>
> -printf("List of user creatable objects:\n");
> +qemu_printf("List of user creatable objects:\n");
> list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
> for (l = list; l != NULL; l = l->next) {
> ObjectClass *oc = OBJECT_CLASS(l->data);
> -printf(" %s\n", object_class_get_name(oc));
> +qemu_printf(" %s\n", object_class_get_name(oc));
> }
> g_slist_free(list);
> }
> @@ -247,12 +248,12 @@ static bool user_creatable_print_type_properites(const
> char *type)
> }
> g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
> if (array->len > 0) {
> -printf("%s options:\n", type);
> +qemu_printf("%s options:\n", type);
> } else {
> -printf("There are no options for %s.\n", type);
> +qemu_printf("There are no options for %s.\n", type);
> }
> for (i = 0; i < array->len; i++) {
> -printf("%s\n", (char *)array->pdata[i]);
> +qemu_printf("%s\n", (char *)array->pdata[i]);
> }
> g_ptr_array_set_free_func(array, g_free);
> g_ptr_array_free(array, true);
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d4001f9c5d..6f5d9ce2fb 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1337,7 +1337,7 @@ ERST
>
> {
> .name = "object_add",
> -.args_type = "object:O",
> +.args_type = "object:S",
> .params = "[qom-type=]type,id=str[,prop=value][,...]",
> .help = "create QOM object",
> .cmd= hmp_object_add,
> --
> 2.29.2
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v2 16/31] qapi/qom: Add ObjectOptions for confidential-guest-support
* Kevin Wolf (kw...@redhat.com) wrote:
> This adds a QAPI schema for the properties of the objects implementing
> the confidential-guest-support interface.
>
> pef-guest and s390x-pv-guest don't have any properties, so they only
> need to be added to the ObjectType enum without adding a new branch to
> ObjectOptions.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi/qom.json | 37 +
> 1 file changed, 37 insertions(+)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index e7184122e9..d5f68b5c89 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -633,6 +633,38 @@
>'base': 'RngProperties',
>'data': { '*filename': 'str' } }
>
> +##
> +# @SevGuestProperties:
> +#
> +# Properties for sev-guest objects.
> +#
> +# @sev-device: SEV device to use (default: "/dev/sev")
> +#
> +# @dh-cert-file: guest owners DH certificate (encoded with base64)
> +#
> +# @session-file: guest owners session parameters (encoded with base64)
> +#
> +# @policy: SEV policy value (default: 0x1)
> +#
> +# @handle: SEV firmware handle (default: 0)
> +#
> +# @cbitpos: C-bit location in page table entry (default: 0)
> +#
> +# @reduced-phys-bits: number of bits in physical addresses that become
> +# unavailable when SEV is enabled
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'SevGuestProperties',
> + 'data': { '*sev-device': 'str',
> +'*dh-cert-file': 'str',
> +'*session-file': 'str',
> +'*policy': 'uint32',
> +'*handle': 'uint32',
> +'*cbitpos': 'uint32',
> +'reduced-phys-bits': 'uint32' },
> + 'if': 'defined(CONFIG_SEV)' }
> +
> ##
> # @ObjectType:
> #
> @@ -661,12 +693,15 @@
> 'memory-backend-file',
> 'memory-backend-memfd',
> 'memory-backend-ram',
> +{'name': 'pef-guest', 'if': 'defined(CONFIG_PSERIES)' },
> 'pr-manager-helper',
> 'rng-builtin',
> 'rng-egd',
> 'rng-random',
> 'secret',
> 'secret_keyring',
> +{'name': 'sev-guest', 'if': 'defined(CONFIG_SEV)' },
> +'s390-pv-guest',
If pef-guest is conditional on PSERIES< shouldn't this be dependent on
s390?
Dave
> 'throttle-group',
> 'tls-creds-anon',
> 'tls-creds-psk',
> @@ -716,6 +751,8 @@
>'rng-random': 'RngRandomProperties',
>'secret': 'SecretProperties',
>'secret_keyring': 'SecretKeyringProperties',
> + 'sev-guest': { 'type': 'SevGuestProperties',
> + 'if': 'defined(CONFIG_SEV)' },
>'throttle-group': 'ThrottleGroupProperties',
>'tls-creds-anon': 'TlsCredsAnonProperties',
>'tls-creds-psk': 'TlsCredsPskProperties',
> --
> 2.29.2
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH 00/14] deprecations: remove many old deprecations
On 2/24/21 3:38 PM, Peter Maydell wrote:
> On Wed, 24 Feb 2021 at 13:21, Daniel P. Berrangé wrote:
>>
>> The following features have been deprecated for well over the 2
>> release cycle we promise
>>
>> ``-usbdevice`` (since 2.10.0)
>> ``-drive file=3Djson:{...{'driver':'file'}}`` (since 3.0)
>> ``-vnc acl`` (since 4.0.0)
>> ``-mon ...,control=3Dreadline,pretty=3Don|off`` (since 4.1)
>
> Are the literal '=3D' here intended ?
No, this is a git-publish bug:
https://github.com/stefanha/git-publish/issues/88
Apparently the fix is not yet backported to Fedora.
Re: [PATCH 00/14] deprecations: remove many old deprecations
On Wed, Feb 24, 2021 at 02:38:43PM +, Peter Maydell wrote:
> On Wed, 24 Feb 2021 at 13:21, Daniel P. Berrangé wrote:
> >
> > The following features have been deprecated for well over the 2
> > release cycle we promise
> >
> > ``-usbdevice`` (since 2.10.0)
> > ``-drive file=3Djson:{...{'driver':'file'}}`` (since 3.0)
> > ``-vnc acl`` (since 4.0.0)
> > ``-mon ...,control=3Dreadline,pretty=3Don|off`` (since 4.1)
>
> Are the literal '=3D' here intended ?
git-publish has done something wierd to the cover letter encoding that
I don't understand.
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: [PATCH 11/14] block: remove 'encryption_key_missing' flag from QAPI
On 24/02/2021 14.11, Daniel P. Berrangé wrote:
This has been hardcoded to "false" since 2.10.0, since secrets required
to unlock block devices are now always provided upfront instead of using
interactive prompts.
Signed-off-by: Daniel P. Berrangé
---
block/qapi.c | 1 -
docs/system/deprecated.rst | 10 ---
docs/system/removed-features.rst | 10 +++
qapi/block-core.json | 8 --
tests/qemu-iotests/184.out | 6 ++--
tests/qemu-iotests/191.out | 48 +++-
tests/qemu-iotests/273.out | 15 --
7 files changed, 33 insertions(+), 65 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 84a0aadc09..3acc118c44 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -62,7 +62,6 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->ro = bs->read_only;
info->drv= g_strdup(bs->drv->format_name);
info->encrypted = bs->encrypted;
-info->encryption_key_missing = false;
info->cache = g_new(BlockdevCacheInfo, 1);
*info->cache = (BlockdevCacheInfo) {
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index cb88fea94f..e746a63edf 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -147,16 +147,6 @@ Use argument ``id`` instead.
Use argument ``id`` instead.
-``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0)
-
-
-Always false.
-
-``query-block`` result ``inserted.encryption_key_missing`` (since 2.10.0)
-'
-
-Always false.
-
``blockdev-add`` empty string argument ``backing`` (since 2.10.0)
'
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index bb6bc8dfc8..583f14f02e 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -112,6 +112,16 @@ chardev client socket with ``wait`` option (removed in 6.0)
Character devices creating sockets in client mode should not specify
the 'wait' field, which is only applicable to sockets in server mode
+``query-named-block-nodes`` result ``encryption_key_missing`` (removed in 6.0)
+''
+
+Always false.
Should that be "Removed with no replacement", too ? (just like the one below)
+``query-block`` result ``inserted.encryption_key_missing`` (removed in 6.0)
+'''
+
+Removed with no replacement
+
Apart from that nit:
Reviewed-by: Thomas Huth
Re: [PATCH 00/14] deprecations: remove many old deprecations
On Wed, 24 Feb 2021 at 13:21, Daniel P. Berrangé wrote:
>
> The following features have been deprecated for well over the 2
> release cycle we promise
>
> ``-usbdevice`` (since 2.10.0)
> ``-drive file=3Djson:{...{'driver':'file'}}`` (since 3.0)
> ``-vnc acl`` (since 4.0.0)
> ``-mon ...,control=3Dreadline,pretty=3Don|off`` (since 4.1)
Are the literal '=3D' here intended ?
thanks
-- PMM
Re: [PATCH 10/14] hw/scsi: remove 'scsi-disk' device
On 24/02/2021 14.11, Daniel P. Berrangé wrote:
The 'scsi-hd' and 'scsi-cd' devices provide suitable alternatives.
Signed-off-by: Daniel P. Berrangé
---
docs/system/deprecated.rst | 9 -
docs/system/removed-features.rst | 6
hw/i386/pc.c | 1 -
hw/scsi/scsi-disk.c | 62
hw/sparc64/sun4u.c | 1 -
scripts/device-crash-test| 1 -
tests/qemu-iotests/051 | 2 --
tests/qemu-iotests/051.pc.out| 10 --
8 files changed, 6 insertions(+), 86 deletions(-)
I see some occurrances of "scsi-disk" in the config files in the
docs/config/ directory, too ... I guess they should also be removed?
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d7c27144ba..cda7df36e3 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -749,7 +749,6 @@ static char *sun4u_fw_dev_path(FWPathProvider *p, BusState
*bus,
DeviceState *dev)
{
PCIDevice *pci;
-int bus_id;
if (!strcmp(object_get_typename(OBJECT(dev)), "pbm-bridge")) {
pci = PCI_DEVICE(dev);
This lonely hunk should be squashed into the previous (ide-disk) patch instead.
Thomas
Re: [PATCH 09/14] hw/ide: remove 'ide-drive' device
On 24/02/2021 14.11, Daniel P. Berrangé wrote:
The 'ide-hd' and 'ide-cd' devices provide suitable alternatives.
Signed-off-by: Daniel P. Berrangé
---
docs/qdev-device-use.txt | 2 +-
docs/system/deprecated.rst | 6 -
docs/system/removed-features.rst | 9
hw/i386/pc.c | 1 -
hw/ide/qdev.c| 38
hw/ppc/mac_newworld.c| 13 ---
hw/ppc/mac_oldworld.c| 13 ---
hw/sparc64/sun4u.c | 14
scripts/device-crash-test| 1 -
softmmu/vl.c | 1 -
tests/qemu-iotests/051 | 2 --
tests/qemu-iotests/051.pc.out| 10 -
12 files changed, 10 insertions(+), 100 deletions(-)
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index 245cdf29c7..2408889334 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -388,7 +388,7 @@ type.
some DEVNAMEs:
default device suppressing DEVNAMEs
-CD-ROM ide-cd, ide-drive, ide-hd, scsi-cd, scsi-hd
+CD-ROM ide-cd, ide-hd, scsi-cd, scsi-hd
floppy floppy, isa-fdc
parallelisa-parallel
serial isa-serial
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index c69887dca8..f5c82a46dc 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -242,12 +242,6 @@ this CPU is also deprecated.
System emulator devices
---
-``ide-drive`` (since 4.2)
-'
-
-The 'ide-drive' device is deprecated. Users should use 'ide-hd' or
-'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed.
-
``scsi-disk`` (since 4.2)
'
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 870a222062..8fd3fafb32 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -213,6 +213,15 @@ This machine has been renamed ``fuloong2e``.
These machine types were very old and likely could not be used for live
migration from old QEMU versions anymore. Use a newer machine type instead.
+System emulator devices
+---
+
+``ide-drive`` (removed in 6.0)
+''
+
+The 'ide-drive' device has been removed. Users should use 'ide-hd' or
+'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed.
+
Related binaries
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8aa85dec54..828122e21e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -342,7 +342,6 @@ GlobalProperty pc_compat_1_4[] = {
{ "scsi-disk", "discard_granularity", "0" },
{ "ide-hd", "discard_granularity", "0" },
{ "ide-cd", "discard_granularity", "0" },
-{ "ide-drive", "discard_granularity", "0" },
{ "virtio-blk-pci", "discard_granularity", "0" },
/* DEV_NVECTORS_UNSPECIFIED as a uint32_t string: */
{ "virtio-serial-pci", "vectors", "0x" },
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 8cd19fa5e9..e70ebc83a0 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -283,20 +283,6 @@ static void ide_cd_realize(IDEDevice *dev, Error **errp)
ide_dev_initfn(dev, IDE_CD, errp);
}
-static void ide_drive_realize(IDEDevice *dev, Error **errp)
-{
-DriveInfo *dinfo = NULL;
-
-warn_report("'ide-drive' is deprecated, "
-"please use 'ide-hd' or 'ide-cd' instead");
-
-if (dev->conf.blk) {
-dinfo = blk_legacy_dinfo(dev->conf.blk);
-}
-
-ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD, errp);
-}
I wonder whether we now could also make the "media" parameter of "-drive" as
deprecated?
Anyway, for this patch:
Reviewed-by: Thomas Huth
Re: [PATCH 04/14] softmmu: remove '-usbdevice' command line option
On Wed, Feb 24, 2021 at 02:58:19PM +0100, Thomas Huth wrote: > On 24/02/2021 14.11, Daniel P. Berrangé wrote: > > This was replaced by the '-device usb-DEV' option. > > > > Signed-off-by: Daniel P. Berrangé > > --- > > docs/system/deprecated.rst | 9 --- > > docs/system/removed-features.rst | 9 +++ > > softmmu/vl.c | 42 > > 3 files changed, 9 insertions(+), 51 deletions(-) > > Last time I tried to remove -usbdevice, there was some concerns that > -usbdevice braille might still be useful for some people, see the thread > that started here: > > https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00651.html > > (and Gerd's summary here: > https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg01520.html ) Urgh, so the current deprecation docs are a bit misleading by saying -usbdevice is directly mapped to -device. > So we might need a new "sugared" option like "-braille" instead before we > can fully remove -usbdevice? ... or we just keep -usbdevice as a bittersweet > remainder? I'm not going to implement new CLI options, and if that's needed, we ought to re-start the clock on the deprecation at that point. So this points towards just removing the deprecation warning that exists today. Or alternatively drop support for -usbdevice, except for the braille type. 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 :|
[PATCH] qemu_monitor: Document qemuMonitorUnregister()
The most important bit is that the caller is expected to pass
locked monitor.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_monitor.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ed35da17e1..73f337a6be 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -850,6 +850,13 @@ qemuMonitorRegister(qemuMonitorPtr mon)
}
+/**
+ * qemuMonitorUnregister:
+ * @mon: monitor object
+ *
+ * Unregister monitor from the event loop. The monitor object
+ * must be locked before calling this function.
+ */
void
qemuMonitorUnregister(qemuMonitorPtr mon)
{
--
2.26.2
Re: [PATCH v2] qemu: Add missing lock in qemuProcessHandleMonitorEOF
On 2/24/21 12:28 PM, Peng Liang wrote: qemuMonitorUnregister will be called in multiple threads (e.g. threads in rpc worker pool and the vm event thread). In some cases, it isn't protected by the monitor lock, which may lead to call g_source_unref more than one time and a use-after-free problem eventually. Add the missing lock in qemuProcessHandleMonitorEOF (which is the only position missing lock of monitor I found). Suggested-by: Michal Privoznik Signed-off-by: Peng Liang --- This patch is v2 of https://listman.redhat.com/archives/libvir-list/2021-February/msg00945.html. v1 -> v2: Locking monitor in qemuProcessHandleMonitorEOF instead of using aotmic function in qemuMonitorUnregister. src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_process.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d25c26343a7f..14e6b1fe9626 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -427,6 +427,7 @@ bool qemuMonitorWasDisposed(void); void qemuMonitorRegister(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); +/* Must be called with monitor locked. */ void qemuMonitorUnregister(qemuMonitorPtr mon) From this it's not very clear which function the comment belongs to. We tend to put comments into .c because that's where tags usually take you first. So you get the memo first. ATTRIBUTE_NONNULL(1); void qemuMonitorClose(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d930ff9a74f6..bfa742577f32 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -318,7 +318,9 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon, /* We don't want this EOF handler to be called over and over while the * thread is waiting for a job. */ +virObjectLock(mon); qemuMonitorUnregister(mon); +virObjectUnlock(mon); /* We don't want any cleanup from EOF handler (or any other * thread) to enter qemu namespace. */ ACK to this hunk. And I'll be pushing it in a few moments. Michal
[PATCH v2 31/31] qom: Drop QemuOpts based interfaces
user_creatable_add_opts() has only a single user left, which is a test
case. Rewrite the test to use user_creatable_add_type() instead (which
is the remaining function that doesn't require a QAPI schema) and drop
the QemuOpts related functions.
Signed-off-by: Kevin Wolf
---
include/qom/object_interfaces.h | 59
qom/object_interfaces.c | 81 -
tests/check-qom-proplist.c | 42 -
3 files changed, 20 insertions(+), 162 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index fb32330901..ac6c33ceac 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -99,51 +99,6 @@ Object *user_creatable_add_type(const char *type, const char
*id,
*/
void user_creatable_add_qapi(ObjectOptions *options, Error **errp);
-/**
- * user_creatable_add_opts:
- * @opts: the object definition
- * @errp: if an error occurs, a pointer to an area to store the error
- *
- * Create an instance of the user creatable object whose type
- * is defined in @opts by the 'qom-type' option, placing it
- * in the object composition tree with name provided by the
- * 'id' field. The remaining options in @opts are used to
- * initialize the object properties.
- *
- * Returns: the newly created object or NULL on error
- */
-Object *user_creatable_add_opts(QemuOpts *opts, Error **errp);
-
-
-/**
- * user_creatable_add_opts_predicate:
- * @type: the QOM type to be added
- *
- * A callback function to determine whether an object
- * of type @type should be created. Instances of this
- * callback should be passed to user_creatable_add_opts_foreach
- */
-typedef bool (*user_creatable_add_opts_predicate)(const char *type);
-
-/**
- * user_creatable_add_opts_foreach:
- * @opaque: a user_creatable_add_opts_predicate callback or NULL
- * @opts: options to create
- * @errp: unused
- *
- * An iterator callback to be used in conjunction with
- * the qemu_opts_foreach() method for creating a list of
- * objects from a set of QemuOpts
- *
- * The @opaque parameter can be passed a user_creatable_add_opts_predicate
- * callback to filter which types of object are created during iteration.
- * When it fails, report the error.
- *
- * Returns: 0 on success, -1 when an error was reported.
- */
-int user_creatable_add_opts_foreach(void *opaque,
-QemuOpts *opts, Error **errp);
-
/**
* user_creatable_parse_str:
* @optarg: the object definition string as passed on the command line
@@ -190,20 +145,6 @@ bool user_creatable_add_from_str(const char *optarg, Error
**errp);
*/
void user_creatable_process_cmdline(const char *optarg);
-/**
- * user_creatable_print_help:
- * @type: the QOM type to be added
- * @opts: options to create
- *
- * Prints help if requested in @type or @opts. Note that if @type is neither
- * "help"/"?" nor a valid user creatable type, no help will be printed
- * regardless of @opts.
- *
- * Returns: true if a help option was found and help was printed, false
- * otherwise.
- */
-bool user_creatable_print_help(const char *type, QemuOpts *opts);
-
/**
* user_creatable_del:
* @id: the unique ID for the object
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 1c29f45b41..25cc54fcd7 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -10,12 +10,9 @@
#include "qapi/qobject-input-visitor.h"
#include "qapi/qobject-output-visitor.h"
#include "qom/object_interfaces.h"
-#include "qemu/help_option.h"
#include "qemu/module.h"
#include "qemu/option.h"
#include "qemu/qemu-print.h"
-#include "qapi/opts-visitor.h"
-#include "qemu/config-file.h"
bool user_creatable_complete(UserCreatable *uc, Error **errp)
{
@@ -131,60 +128,6 @@ void user_creatable_add_qapi(ObjectOptions *options, Error
**errp)
visit_free(v);
}
-Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
-{
-Visitor *v;
-QDict *pdict;
-Object *obj;
-const char *id = qemu_opts_id(opts);
-char *type = qemu_opt_get_del(opts, "qom-type");
-
-if (!type) {
-error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
-return NULL;
-}
-if (!id) {
-error_setg(errp, QERR_MISSING_PARAMETER, "id");
-qemu_opt_set(opts, "qom-type", type, _abort);
-g_free(type);
-return NULL;
-}
-
-qemu_opts_set_id(opts, NULL);
-pdict = qemu_opts_to_qdict(opts, NULL);
-
-v = opts_visitor_new(opts);
-obj = user_creatable_add_type(type, id, pdict, v, errp);
-visit_free(v);
-
-qemu_opts_set_id(opts, (char *) id);
-qemu_opt_set(opts, "qom-type", type, _abort);
-g_free(type);
-qobject_unref(pdict);
-return obj;
-}
-
-
-int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
-{
-bool (*type_opt_predicate)(const char *, QemuOpts *) = opaque;
-Object *obj = NULL;
-const char *type;
-
-type = qemu_opt_get(opts,
[PATCH v2 23/31] qom: Factor out user_creatable_process_cmdline()
The implementation for --object can be shared between
qemu-storage-daemon and other binaries, so move it into a function in
qom/object_interfaces.c that is accessible from everywhere.
This also requires moving the implementation of qmp_object_add() into a
new user_creatable_add_qapi(), because qom/qom-qmp-cmds.c is not linked
for tools.
user_creatable_print_help_from_qdict() can become static now.
Signed-off-by: Kevin Wolf
---
include/qom/object_interfaces.h | 41 +++
qom/object_interfaces.c | 50 +++-
qom/qom-qmp-cmds.c | 20 +--
storage-daemon/qemu-storage-daemon.c | 22 +---
4 files changed, 79 insertions(+), 54 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 5299603f50..1e6c51b541 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -2,6 +2,7 @@
#define OBJECT_INTERFACES_H
#include "qom/object.h"
+#include "qapi/qapi-types-qom.h"
#include "qapi/visitor.h"
#define TYPE_USER_CREATABLE "user-creatable"
@@ -86,6 +87,18 @@ Object *user_creatable_add_type(const char *type, const char
*id,
const QDict *qdict,
Visitor *v, Error **errp);
+/**
+ * user_creatable_add_qapi:
+ * @options: the object definition
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object according to the
+ * options passed in @opts as described in the QAPI schema documentation.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+void user_creatable_add_qapi(ObjectOptions *options, Error **errp);
+
/**
* user_creatable_add_opts:
* @opts: the object definition
@@ -131,6 +144,21 @@ typedef bool (*user_creatable_add_opts_predicate)(const
char *type);
int user_creatable_add_opts_foreach(void *opaque,
QemuOpts *opts, Error **errp);
+/**
+ * user_creatable_process_cmdline:
+ * @optarg: the object definition string as passed on the command line
+ *
+ * Create an instance of the user creatable object by parsing optarg
+ * with a keyval parser and implicit key 'qom-type', converting the
+ * result to ObjectOptions and calling into qmp_object_add().
+ *
+ * If a help option is given, print help instead and exit.
+ *
+ * This function is only meant to be called during command line parsing.
+ * It exits the process on failure or after printing help.
+ */
+void user_creatable_process_cmdline(const char *optarg);
+
/**
* user_creatable_print_help:
* @type: the QOM type to be added
@@ -145,19 +173,6 @@ int user_creatable_add_opts_foreach(void *opaque,
*/
bool user_creatable_print_help(const char *type, QemuOpts *opts);
-/**
- * user_creatable_print_help_from_qdict:
- * @args: options to create
- *
- * Prints help considering the other options given in @args (if "qom-type" is
- * given and valid, print properties for the type, otherwise print valid types)
- *
- * In contrast to user_creatable_print_help(), this function can't return that
- * no help was requested. It should only be called if we know that help is
- * requested and it will always print some help.
- */
-void user_creatable_print_help_from_qdict(QDict *args);
-
/**
* user_creatable_del:
* @id: the unique ID for the object
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 7d8a4b77b8..efb48249d5 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -2,10 +2,13 @@
#include "qemu/cutils.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qjson.h"
#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
#include "qom/object_interfaces.h"
#include "qemu/help_option.h"
#include "qemu/module.h"
@@ -104,6 +107,29 @@ out:
return obj;
}
+void user_creatable_add_qapi(ObjectOptions *options, Error **errp)
+{
+Visitor *v;
+QObject *qobj;
+QDict *props;
+Object *obj;
+
+v = qobject_output_visitor_new();
+visit_type_ObjectOptions(v, NULL, , _abort);
+visit_complete(v, );
+visit_free(v);
+
+props = qobject_to(QDict, qobj);
+qdict_del(props, "qom-type");
+qdict_del(props, "id");
+
+v = qobject_input_visitor_new(QOBJECT(props));
+obj = user_creatable_add_type(ObjectType_str(options->qom_type),
+ options->id, props, v, errp);
+object_unref(obj);
+visit_free(v);
+}
+
Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
{
Visitor *v;
@@ -247,7 +273,7 @@ bool user_creatable_print_help(const char *type, QemuOpts
*opts)
return false;
}
-void user_creatable_print_help_from_qdict(QDict *args)
+static void user_creatable_print_help_from_qdict(QDict *args)
{
const char *type = qdict_get_try_str(args,
[PATCH v2 22/31] qom: Remove user_creatable_add_dict()
This function is now unused and can be removed.
Signed-off-by: Kevin Wolf
---
include/qom/object_interfaces.h | 18 --
qom/object_interfaces.c | 32
2 files changed, 50 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 9b9938b8c0..5299603f50 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -86,24 +86,6 @@ Object *user_creatable_add_type(const char *type, const char
*id,
const QDict *qdict,
Visitor *v, Error **errp);
-/**
- * user_creatable_add_dict:
- * @qdict: the object definition
- * @keyval: if true, use a keyval visitor for processing @qdict (i.e.
- * assume that all @qdict values are strings); otherwise, use
- * the normal QObject visitor (i.e. assume all @qdict values
- * have the QType expected by the QOM object type)
- * @errp: if an error occurs, a pointer to an area to store the error
- *
- * Create an instance of the user creatable object that is defined by
- * @qdict. The object type is taken from the QDict key 'qom-type', its
- * ID from the key 'id'. The remaining entries in @qdict are used to
- * initialize the object properties.
- *
- * Returns: %true on success, %false on failure.
- */
-bool user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp);
-
/**
* user_creatable_add_opts:
* @opts: the object definition
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index b9a99c8bf4..7d8a4b77b8 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -104,38 +104,6 @@ out:
return obj;
}
-bool user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp)
-{
-Visitor *v;
-Object *obj;
-g_autofree char *type = NULL;
-g_autofree char *id = NULL;
-
-type = g_strdup(qdict_get_try_str(qdict, "qom-type"));
-if (!type) {
-error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
-return false;
-}
-qdict_del(qdict, "qom-type");
-
-id = g_strdup(qdict_get_try_str(qdict, "id"));
-if (!id) {
-error_setg(errp, QERR_MISSING_PARAMETER, "id");
-return false;
-}
-qdict_del(qdict, "id");
-
-if (keyval) {
-v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
-} else {
-v = qobject_input_visitor_new(QOBJECT(qdict));
-}
-obj = user_creatable_add_type(type, id, qdict, v, errp);
-visit_free(v);
-object_unref(obj);
-return !!obj;
-}
-
Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
{
Visitor *v;
--
2.29.2
[PATCH v2 19/31] qapi/qom: QAPIfy object-add
This converts object-add from 'gen': false to the ObjectOptions QAPI
type. As an immediate benefit, clients can now use QAPI schema
introspection for user creatable QOM objects.
It is also the first step towards making the QAPI schema the only
external interface for the creation of user creatable objects. Once all
other places (HMP and command lines of the system emulator and all
tools) go through QAPI, too, some object implementations can be
simplified because some checks (e.g. that mandatory options are set) are
already performed by QAPI, and in another step, QOM boilerplate code
could be generated from the schema.
Signed-off-by: Kevin Wolf
---
qapi/qom.json| 11 +--
include/qom/object_interfaces.h | 7 ---
hw/block/xen-block.c | 16
monitor/misc.c | 2 --
qom/qom-qmp-cmds.c | 25 +++--
storage-daemon/qemu-storage-daemon.c | 2 --
6 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index 6793342e81..e5b219df58 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -839,13 +839,6 @@
#
# Create a QOM object.
#
-# @qom-type: the class name for the object to be created
-#
-# @id: the name of the new object
-#
-# Additional arguments depend on qom-type and are passed to the backend
-# unchanged.
-#
# Returns: Nothing on success
# Error if @qom-type is not a valid class name
#
@@ -859,9 +852,7 @@
# <- { "return": {} }
#
##
-{ 'command': 'object-add',
- 'data': {'qom-type': 'str', 'id': 'str'},
- 'gen': false } # so we can get the additional arguments
+{ 'command': 'object-add', 'data': 'ObjectOptions', 'boxed': true }
##
# @object-del:
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 07d5cc8832..9b9938b8c0 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -196,11 +196,4 @@ bool user_creatable_del(const char *id, Error **errp);
*/
void user_creatable_cleanup(void);
-/**
- * qmp_object_add:
- *
- * QMP command handler for object-add. See the QAPI schema for documentation.
- */
-void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp);
-
#endif
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index a3b69e2709..ac82d54063 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -836,17 +836,17 @@ static XenBlockIOThread *xen_block_iothread_create(const
char *id,
{
ERRP_GUARD();
XenBlockIOThread *iothread = g_new(XenBlockIOThread, 1);
-QDict *opts;
-QObject *ret_data = NULL;
+ObjectOptions *opts;
iothread->id = g_strdup(id);
-opts = qdict_new();
-qdict_put_str(opts, "qom-type", TYPE_IOTHREAD);
-qdict_put_str(opts, "id", id);
-qmp_object_add(opts, _data, errp);
-qobject_unref(opts);
-qobject_unref(ret_data);
+opts = g_new(ObjectOptions, 1);
+*opts = (ObjectOptions) {
+.qom_type = OBJECT_TYPE_IOTHREAD,
+.id = g_strdup(id),
+};
+qmp_object_add(opts, errp);
+qapi_free_ObjectOptions(opts);
if (*errp) {
g_free(iothread->id);
diff --git a/monitor/misc.c b/monitor/misc.c
index a7650ed747..42efd9e2ab 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -235,8 +235,6 @@ static void monitor_init_qmp_commands(void)
qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
qmp_register_command(_commands, "device_add", qmp_device_add,
QCO_NO_OPTIONS);
-qmp_register_command(_commands, "object-add", qmp_object_add,
- QCO_NO_OPTIONS);
QTAILQ_INIT(_cap_negotiation_commands);
qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 19fd5e117f..e577a96adf 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -19,8 +19,11 @@
#include "qapi/error.h"
#include "qapi/qapi-commands-qdev.h"
#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
#include "qemu/cutils.h"
#include "qom/object_interfaces.h"
#include "qom/qom-qobject.h"
@@ -223,9 +226,27 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char
*typename,
return prop_list;
}
-void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp)
+void qmp_object_add(ObjectOptions *options, Error **errp)
{
-user_creatable_add_dict(qdict, false, errp);
+Visitor *v;
+QObject *qobj;
+QDict *props;
+Object *obj;
+
+v = qobject_output_visitor_new();
+visit_type_ObjectOptions(v, NULL, , _abort);
+visit_complete(v, );
+visit_free(v);
+
+props = qobject_to(QDict, qobj);
+qdict_del(props, "qom-type");
+qdict_del(props, "id");
+
+v = qobject_input_visitor_new(QOBJECT(props));
+obj =
[PATCH v2 27/31] qom: Add user_creatable_add_from_str()
This is a version of user_creatable_process_cmdline() with an Error
parameter that never calls exit() and is therefore usable in HMP.
Signed-off-by: Kevin Wolf
---
include/qom/object_interfaces.h | 16
qom/object_interfaces.c | 29 -
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 1e6c51b541..07511e6cff 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -144,6 +144,22 @@ typedef bool (*user_creatable_add_opts_predicate)(const
char *type);
int user_creatable_add_opts_foreach(void *opaque,
QemuOpts *opts, Error **errp);
+/**
+ * user_creatable_add_from_str:
+ * @optarg: the object definition string as passed on the command line
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object by parsing optarg
+ * with a keyval parser and implicit key 'qom-type', converting the
+ * result to ObjectOptions and calling into qmp_object_add().
+ *
+ * If a help option is given, print help instead.
+ *
+ * Returns: true when an object was successfully created, false when an error
+ * occurred (*errp is set then) or help was printed (*errp is not set).
+ */
+bool user_creatable_add_from_str(const char *optarg, Error **errp);
+
/**
* user_creatable_process_cmdline:
* @optarg: the object definition string as passed on the command line
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index efb48249d5..54f0dadfea 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -282,26 +282,45 @@ static void user_creatable_print_help_from_qdict(QDict
*args)
}
}
-void user_creatable_process_cmdline(const char *optarg)
+bool user_creatable_add_from_str(const char *optarg, Error **errp)
{
+ERRP_GUARD();
QDict *args;
bool help;
Visitor *v;
ObjectOptions *options;
-args = keyval_parse(optarg, "qom-type", , _fatal);
+args = keyval_parse(optarg, "qom-type", , errp);
+if (*errp) {
+return false;
+}
if (help) {
user_creatable_print_help_from_qdict(args);
-exit(EXIT_SUCCESS);
+qobject_unref(args);
+return false;
}
v = qobject_input_visitor_new_keyval(QOBJECT(args));
-visit_type_ObjectOptions(v, NULL, , _fatal);
+visit_type_ObjectOptions(v, NULL, , errp);
visit_free(v);
qobject_unref(args);
-user_creatable_add_qapi(options, _fatal);
+if (*errp) {
+goto out;
+}
+
+user_creatable_add_qapi(options, errp);
+out:
qapi_free_ObjectOptions(options);
+return !*errp;
+}
+
+void user_creatable_process_cmdline(const char *optarg)
+{
+if (!user_creatable_add_from_str(optarg, _fatal)) {
+/* Help was printed */
+exit(EXIT_SUCCESS);
+}
}
bool user_creatable_del(const char *id, Error **errp)
--
2.29.2
Re: [PATCH 04/14] softmmu: remove '-usbdevice' command line option
On 24/02/2021 14.11, Daniel P. Berrangé wrote: This was replaced by the '-device usb-DEV' option. Signed-off-by: Daniel P. Berrangé --- docs/system/deprecated.rst | 9 --- docs/system/removed-features.rst | 9 +++ softmmu/vl.c | 42 3 files changed, 9 insertions(+), 51 deletions(-) Last time I tried to remove -usbdevice, there was some concerns that -usbdevice braille might still be useful for some people, see the thread that started here: https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00651.html (and Gerd's summary here: https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg01520.html ) So we might need a new "sugared" option like "-braille" instead before we can fully remove -usbdevice? ... or we just keep -usbdevice as a bittersweet remainder? Thomas
[PATCH v2 28/31] hmp: QAPIfy object_add
This switches the HMP command object_add from a QemuOpts-based parser to
user_creatable_add_from_str() which uses a keyval parser and enforces
the QAPI schema.
Apart from being a cleanup, this makes non-scalar properties and help
accessible. In order for help to be printed to the monitor instead of
stdout, the printf() calls in the help functions are changed to
qemu_printf().
Signed-off-by: Kevin Wolf
---
monitor/hmp-cmds.c | 17 ++---
qom/object_interfaces.c | 11 ++-
hmp-commands.hx | 2 +-
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 3c88a4faef..652cf9ff21 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
+const char *options = qdict_get_str(qdict, "object");
Error *err = NULL;
-QemuOpts *opts;
-Object *obj = NULL;
-
-opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, );
-if (err) {
-goto end;
-}
-obj = user_creatable_add_opts(opts, );
-qemu_opts_del(opts);
-
-end:
+user_creatable_add_from_str(options, );
hmp_handle_error(mon, err);
-
-if (obj) {
-object_unref(obj);
-}
}
void hmp_getfd(Monitor *mon, const QDict *qdict)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 54f0dadfea..c4982dd7a0 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -13,6 +13,7 @@
#include "qemu/help_option.h"
#include "qemu/module.h"
#include "qemu/option.h"
+#include "qemu/qemu-print.h"
#include "qapi/opts-visitor.h"
#include "qemu/config-file.h"
@@ -212,11 +213,11 @@ static void user_creatable_print_types(void)
{
GSList *l, *list;
-printf("List of user creatable objects:\n");
+qemu_printf("List of user creatable objects:\n");
list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
for (l = list; l != NULL; l = l->next) {
ObjectClass *oc = OBJECT_CLASS(l->data);
-printf(" %s\n", object_class_get_name(oc));
+qemu_printf(" %s\n", object_class_get_name(oc));
}
g_slist_free(list);
}
@@ -247,12 +248,12 @@ static bool user_creatable_print_type_properites(const
char *type)
}
g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
if (array->len > 0) {
-printf("%s options:\n", type);
+qemu_printf("%s options:\n", type);
} else {
-printf("There are no options for %s.\n", type);
+qemu_printf("There are no options for %s.\n", type);
}
for (i = 0; i < array->len; i++) {
-printf("%s\n", (char *)array->pdata[i]);
+qemu_printf("%s\n", (char *)array->pdata[i]);
}
g_ptr_array_set_free_func(array, g_free);
g_ptr_array_free(array, true);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d4001f9c5d..6f5d9ce2fb 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1337,7 +1337,7 @@ ERST
{
.name = "object_add",
-.args_type = "object:O",
+.args_type = "object:S",
.params = "[qom-type=]type,id=str[,prop=value][,...]",
.help = "create QOM object",
.cmd= hmp_object_add,
--
2.29.2
[PATCH v2 25/31] qemu-img: Use user_creatable_process_cmdline() for --object
This switches qemu-img from a QemuOpts-based parser for --object to
user_creatable_process_cmdline() which uses a keyval parser and enforces
the QAPI schema.
Apart from being a cleanup, this makes non-scalar properties accessible.
Signed-off-by: Kevin Wolf
---
qemu-img.c | 239 -
1 file changed, 33 insertions(+), 206 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index e2952fe955..ebf8661e2a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -226,23 +226,6 @@ static void QEMU_NORETURN help(void)
exit(EXIT_SUCCESS);
}
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
-static bool qemu_img_object_print_help(const char *type, QemuOpts *opts)
-{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-return true;
-}
-
/*
* Is @optarg safe for accumulate_options()?
* It is when multiple of them can be joined together separated by ','.
@@ -566,14 +549,9 @@ static int img_create(int argc, char **argv)
case 'u':
flags |= BDRV_O_NO_BACKING;
break;
-case OPTION_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-goto fail;
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
}
}
@@ -589,12 +567,6 @@ static int img_create(int argc, char **argv)
}
optind++;
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_img_object_print_help, _fatal)) {
-goto fail;
-}
-
/* Get image size, if specified */
if (optind < argc) {
int64_t sval;
@@ -804,14 +776,9 @@ static int img_check(int argc, char **argv)
case 'U':
force_share = true;
break;
-case OPTION_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-return 1;
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
@@ -831,12 +798,6 @@ static int img_check(int argc, char **argv)
return 1;
}
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_img_object_print_help, _fatal)) {
-return 1;
-}
-
ret = bdrv_parse_cache_mode(cache, , );
if (ret < 0) {
error_report("Invalid source cache option: %s", cache);
@@ -1034,14 +995,9 @@ static int img_commit(int argc, char **argv)
return 1;
}
break;
-case OPTION_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-return 1;
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
@@ -1058,12 +1014,6 @@ static int img_commit(int argc, char **argv)
}
filename = argv[optind++];
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_img_object_print_help, _fatal)) {
-return 1;
-}
-
flags = BDRV_O_RDWR | BDRV_O_UNMAP;
ret = bdrv_parse_cache_mode(cache, , );
if (ret < 0) {
@@ -1423,15 +1373,9 @@ static int img_compare(int argc, char **argv)
case 'U':
force_share = true;
break;
-case OPTION_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-ret = 2;
-goto out4;
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
@@ -1450,13 +1394,6 @@ static int img_compare(int argc, char **argv)
filename1 = argv[optind++];
filename2 = argv[optind++];
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_img_object_print_help, _fatal)) {
-ret = 2;
-goto out4;
-}
-
/* Initialize before goto
[PATCH v2 24/31] qemu-io: Use user_creatable_process_cmdline() for --object
This switches qemu-io from a QemuOpts-based parser for --object to
user_creatable_process_cmdline() which uses a keyval parser and enforces
the QAPI schema.
Apart from being a cleanup, this makes non-scalar properties accessible.
Signed-off-by: Kevin Wolf
---
qemu-io.c | 33 +++--
1 file changed, 3 insertions(+), 30 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index ac88d8bd40..bf902302e9 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -477,23 +477,6 @@ enum {
OPTION_IMAGE_OPTS = 257,
};
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
-static bool qemu_io_object_print_help(const char *type, QemuOpts *opts)
-{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-return true;
-}
-
static QemuOptsList file_opts = {
.name = "file",
.implied_opt_name = "file",
@@ -550,7 +533,6 @@ int main(int argc, char **argv)
qcrypto_init(_fatal);
module_call_init(MODULE_INIT_QOM);
-qemu_add_opts(_object_opts);
qemu_add_opts(_trace_opts);
bdrv_init();
@@ -612,14 +594,9 @@ int main(int argc, char **argv)
case 'U':
force_share = true;
break;
-case OPTION_OBJECT: {
-QemuOpts *qopts;
-qopts = qemu_opts_parse_noisily(_object_opts,
-optarg, true);
-if (!qopts) {
-exit(1);
-}
-} break;
+case OPTION_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case OPTION_IMAGE_OPTS:
imageOpts = true;
break;
@@ -644,10 +621,6 @@ int main(int argc, char **argv)
exit(1);
}
-qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_io_object_print_help, _fatal);
-
if (!trace_init_backends()) {
exit(1);
}
--
2.29.2
[PATCH v2 30/31] vl: QAPIfy -object
This switches the system emulator from a QemuOpts-based parser for
-object to user_creatable_parse_str() which uses a keyval parser and
enforces the QAPI schema.
Apart from being a cleanup, this makes non-scalar properties accessible.
This adopts a similar model as -blockdev uses: When parsing the option,
create the ObjectOptions and queue them. At the later point where we
used to create objects for the collected QemuOpts, the ObjectOptions
queue is processed instead.
A complication compared to -blockdev is that object definitions are
supported in -readconfig and -writeconfig.
After this patch, -readconfig still works, though it still goes through
the QemuOpts parser, which means that improvements like non-scalar
properties are still not available in config files.
-writeconfig stops working for -object. Tough luck. It has never
supported all options (not even the common ones), so supporting one less
isn't the end of the world. As object definitions from -readconfig still
go through QemuOpts, they are still included in -writeconfig output,
which at least prevents destroying your existing configuration when you
just wanted to add another option.
Signed-off-by: Kevin Wolf
---
softmmu/vl.c | 109 +++
1 file changed, 84 insertions(+), 25 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b219ce1f35..205c254542 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -113,6 +113,7 @@
#include "sysemu/replay.h"
#include "qapi/qapi-events-run-state.h"
#include "qapi/qapi-visit-block-core.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qapi-visit-ui.h"
#include "qapi/qapi-commands-block-core.h"
#include "qapi/qapi-commands-migration.h"
@@ -132,6 +133,14 @@ typedef struct BlockdevOptionsQueueEntry {
typedef QSIMPLEQ_HEAD(, BlockdevOptionsQueueEntry) BlockdevOptionsQueue;
+typedef struct ObjectOptionsQueueEntry {
+ObjectOptions *options;
+Location loc;
+QTAILQ_ENTRY(ObjectOptionsQueueEntry) next;
+} ObjectOptionsQueueEntry;
+
+typedef QTAILQ_HEAD(, ObjectOptionsQueueEntry) ObjectOptionsQueue;
+
static const char *cpu_option;
static const char *mem_path;
static const char *incoming;
@@ -143,6 +152,7 @@ static int snapshot;
static bool preconfig_requested;
static QemuPluginList plugin_list = QTAILQ_HEAD_INITIALIZER(plugin_list);
static BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
+static ObjectOptionsQueue obj_queue = QTAILQ_HEAD_INITIALIZER(obj_queue);
static bool nographic = false;
static int mem_prealloc; /* force preallocation of physical target memory */
static ram_addr_t ram_size;
@@ -1691,12 +1701,9 @@ static int machine_set_property(void *opaque,
* cannot be created here, as it depends on the chardev
* already existing.
*/
-static bool object_create_early(const char *type, QemuOpts *opts)
+static bool object_create_early(ObjectOptions *options)
{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-
+const char *type = ObjectType_str(options->qom_type);
/*
* Objects should not be made "delayed" without a reason. If you
* add one, state the reason in a comment!
@@ -1744,6 +1751,56 @@ static bool object_create_early(const char *type,
QemuOpts *opts)
return true;
}
+static void object_queue_create(bool early)
+{
+ObjectOptionsQueueEntry *entry, *next;
+
+QTAILQ_FOREACH_SAFE(entry, _queue, next, next) {
+if (early != object_create_early(entry->options)) {
+continue;
+}
+QTAILQ_REMOVE(_queue, entry, next);
+loc_push_restore(>loc);
+user_creatable_add_qapi(entry->options, _fatal);
+loc_pop(>loc);
+qapi_free_ObjectOptions(entry->options);
+g_free(entry);
+}
+}
+
+/*
+ * -readconfig still parses things into QemuOpts. Convert any such
+ * configurations to an ObjectOptionsQueueEntry.
+ *
+ * This is more restricted than the normal -object parser because QemuOpts
+ * parsed things, so no support for non-scalar properties. Help is also not
+ * supported (but this shouldn't be requested in a config file anyway).
+ */
+static int object_readconfig_to_qapi(void *opaque, QemuOpts *opts, Error
**errp)
+{
+ERRP_GUARD();
+ObjectOptionsQueueEntry *entry;
+ObjectOptions *options;
+QDict *args = qemu_opts_to_qdict(opts, NULL);
+Visitor *v;
+
+v = qobject_input_visitor_new_keyval(QOBJECT(args));
+visit_type_ObjectOptions(v, NULL, , errp);
+visit_free(v);
+qobject_unref(args);
+
+if (*errp) {
+return -1;
+}
+
+entry = g_new0(ObjectOptionsQueueEntry, 1);
+entry->options = options;
+loc_save(>loc);
+QTAILQ_INSERT_TAIL(_queue, entry, next);
+
+return 0;
+}
+
static void qemu_apply_machine_options(void)
{
MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
@@ -1816,8 +1873,8 @@ static void qemu_create_early_backends(void)
}
[PATCH v2 21/31] qemu-storage-daemon: Implement --object with qmp_object_add()
This QAPIfies --object and ensures that QMP and the command line option
behave the same.
Signed-off-by: Kevin Wolf
---
storage-daemon/qemu-storage-daemon.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/storage-daemon/qemu-storage-daemon.c
b/storage-daemon/qemu-storage-daemon.c
index d8d172cc60..0dfb9c1448 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -38,6 +38,7 @@
#include "qapi/qapi-visit-block-core.h"
#include "qapi/qapi-visit-block-export.h"
#include "qapi/qapi-visit-control.h"
+#include "qapi/qapi-visit-qom.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qstring.h"
#include "qapi/qobject-input-visitor.h"
@@ -130,15 +131,6 @@ enum {
extern QemuOptsList qemu_chardev_opts;
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
static void init_qmp_commands(void)
{
qmp_init_marshal(_commands);
@@ -263,14 +255,22 @@ static void process_options(int argc, char *argv[])
{
QDict *args;
bool help;
+Visitor *v;
+ObjectOptions *options;
args = keyval_parse(optarg, "qom-type", , _fatal);
if (help) {
user_creatable_print_help_from_qdict(args);
exit(EXIT_SUCCESS);
}
-user_creatable_add_dict(args, true, _fatal);
+
+v = qobject_input_visitor_new_keyval(QOBJECT(args));
+visit_type_ObjectOptions(v, NULL, , _fatal);
+visit_free(v);
qobject_unref(args);
+
+qmp_object_add(options, _fatal);
+qapi_free_ObjectOptions(options);
break;
}
default:
@@ -295,7 +295,6 @@ int main(int argc, char *argv[])
module_call_init(MODULE_INIT_QOM);
module_call_init(MODULE_INIT_TRACE);
-qemu_add_opts(_object_opts);
qemu_add_opts(_trace_opts);
qcrypto_init(_fatal);
bdrv_init();
--
2.29.2
[PATCH v2 07/31] qapi/qom: Add ObjectOptions for memory-backend-*
This adds a QAPI schema for the properties of the memory-backend-*
objects.
HostMemPolicy has to be moved to an include file that can be used by the
storage daemon, too, because ObjectOptions must be the same in all
binaries if we don't want to compile the whole code multiple times.
Signed-off-by: Kevin Wolf
---
qapi/common.json | 20
qapi/machine.json | 22 +
qapi/qom.json | 118 +-
3 files changed, 138 insertions(+), 22 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index 716712d4b3..2dad4fadc3 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -145,3 +145,23 @@
##
{ 'enum': 'PCIELinkWidth',
'data': [ '1', '2', '4', '8', '12', '16', '32' ] }
+
+##
+# @HostMemPolicy:
+#
+# Host memory policy types
+#
+# @default: restore default policy, remove any nondefault policy
+#
+# @preferred: set the preferred host nodes for allocation
+#
+# @bind: a strict policy that restricts memory allocation to the
+#host nodes specified
+#
+# @interleave: memory allocations are interleaved across the set
+# of host nodes specified
+#
+# Since: 2.1
+##
+{ 'enum': 'HostMemPolicy',
+ 'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
diff --git a/qapi/machine.json b/qapi/machine.json
index 330189efe3..4322aee782 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -8,6 +8,8 @@
# = Machines
##
+{ 'include': 'common.json' }
+
##
# @SysEmuTarget:
#
@@ -897,26 +899,6 @@
'policy': 'HmatCacheWritePolicy',
'line': 'uint16' }}
-##
-# @HostMemPolicy:
-#
-# Host memory policy types
-#
-# @default: restore default policy, remove any nondefault policy
-#
-# @preferred: set the preferred host nodes for allocation
-#
-# @bind: a strict policy that restricts memory allocation to the
-#host nodes specified
-#
-# @interleave: memory allocations are interleaved across the set
-# of host nodes specified
-#
-# Since: 2.1
-##
-{ 'enum': 'HostMemPolicy',
- 'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
-
##
# @memsave:
#
diff --git a/qapi/qom.json b/qapi/qom.json
index a6a5049707..1a869006a1 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -5,6 +5,7 @@
# See the COPYING file in the top-level directory.
{ 'include': 'authz.json' }
+{ 'include': 'common.json' }
##
# = QEMU Object Model (QOM)
@@ -272,6 +273,113 @@
'*poll-grow': 'int',
'*poll-shrink': 'int' } }
+##
+# @MemoryBackendProperties:
+#
+# Properties for objects of classes derived from memory-backend.
+#
+# @merge: if true, mark the memory as mergeable (default depends on the machine
+# type)
+#
+# @dump: if true, include the memory in core dumps (default depends on the
+#machine type)
+#
+# @host-nodes: the list of NUMA host nodes to bind the memory to
+#
+# @policy: the NUMA policy (default: 'default')
+#
+# @prealloc: if true, preallocate memory (default: false)
+#
+# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
+#
+# @share: if false, the memory is private to QEMU; if true, it is shared
+# (default: false)
+#
+# @size: size of the memory region in bytes
+#
+# @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
+#for ramblock-id. Disable this for 4.0
+#machine types or older to allow
+#migration with newer QEMU versions.
+#(default: false generally, but true
+#for machine types <= 4.0)
+#
+# Since: 2.1
+##
+{ 'struct': 'MemoryBackendProperties',
+ 'data': { '*dump': 'bool',
+'*host-nodes': ['uint16'],
+'*merge': 'bool',
+'*policy': 'HostMemPolicy',
+'*prealloc': 'bool',
+'*prealloc-threads': 'uint32',
+'*share': 'bool',
+'size': 'size',
+'*x-use-canonical-path-for-ramblock-id': 'bool' } }
+
+##
+# @MemoryBackendFileProperties:
+#
+# Properties for memory-backend-file objects.
+#
+# @align: the base address alignment when QEMU mmap(2) @mem-path. Some
+# backend store specified by @mem-path requires an alignment different
+# than the default one used by QEMU, e.g. the device DAX /dev/dax0.0
+# requires 2M alignment rather than 4K. In such cases, users can
+# specify the required alignment via this option.
+# 0 selects a default alignment (currently the page size). (default: 0)
+#
+# @discard-data: if true, the file contents can be destroyed when QEMU exits,
+#to avoid unnecessarily flushing data to the backing file. Note
+#that ``discard-data`` is only an optimization, and QEMU might
+#not discard file contents if it aborts unexpectedly or is
+#terminated using SIGKILL. (default: false)
+#
+# @mem-path:
[PATCH v2 29/31] qom: Add user_creatable_parse_str()
The system emulator has a more complicated way of handling command line
options in that it reorders options before it processes them. This means
that parsing object options and creating the object happen at two
different points. Split the parsing part into a separate function that
can be reused by the system emulator command line.
Signed-off-by: Kevin Wolf
---
include/qom/object_interfaces.h | 15 +++
qom/object_interfaces.c | 20 ++--
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 07511e6cff..fb32330901 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -144,6 +144,21 @@ typedef bool (*user_creatable_add_opts_predicate)(const
char *type);
int user_creatable_add_opts_foreach(void *opaque,
QemuOpts *opts, Error **errp);
+/**
+ * user_creatable_parse_str:
+ * @optarg: the object definition string as passed on the command line
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Parses the option for the user creatable object with a keyval parser and
+ * implicit key 'qom-type', converting the result to ObjectOptions.
+ *
+ * If a help option is given, print help instead.
+ *
+ * Returns: ObjectOptions on success, NULL when an error occurred (*errp is set
+ * then) or help was printed (*errp is not set).
+ */
+ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp);
+
/**
* user_creatable_add_from_str:
* @optarg: the object definition string as passed on the command line
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index c4982dd7a0..1c29f45b41 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -283,7 +283,7 @@ static void user_creatable_print_help_from_qdict(QDict
*args)
}
}
-bool user_creatable_add_from_str(const char *optarg, Error **errp)
+ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
{
ERRP_GUARD();
QDict *args;
@@ -293,12 +293,12 @@ bool user_creatable_add_from_str(const char *optarg,
Error **errp)
args = keyval_parse(optarg, "qom-type", , errp);
if (*errp) {
-return false;
+return NULL;
}
if (help) {
user_creatable_print_help_from_qdict(args);
qobject_unref(args);
-return false;
+return NULL;
}
v = qobject_input_visitor_new_keyval(QOBJECT(args));
@@ -306,12 +306,20 @@ bool user_creatable_add_from_str(const char *optarg,
Error **errp)
visit_free(v);
qobject_unref(args);
-if (*errp) {
-goto out;
+return options;
+}
+
+bool user_creatable_add_from_str(const char *optarg, Error **errp)
+{
+ERRP_GUARD();
+ObjectOptions *options;
+
+options = user_creatable_parse_str(optarg, errp);
+if (!options) {
+return false;
}
user_creatable_add_qapi(options, errp);
-out:
qapi_free_ObjectOptions(options);
return !*errp;
}
--
2.29.2
[PATCH v2 26/31] qemu-nbd: Use user_creatable_process_cmdline() for --object
This switches qemu-nbd from a QemuOpts-based parser for --object to
user_creatable_process_cmdline() which uses a keyval parser and enforces
the QAPI schema.
Apart from being a cleanup, this makes non-scalar properties accessible.
Signed-off-by: Kevin Wolf
---
qemu-nbd.c | 34 +++---
1 file changed, 3 insertions(+), 31 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index b1b9430a8f..93ef4e288f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -401,24 +401,6 @@ static QemuOptsList file_opts = {
},
};
-static QemuOptsList qemu_object_opts = {
-.name = "object",
-.implied_opt_name = "qom-type",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
-.desc = {
-{ }
-},
-};
-
-static bool qemu_nbd_object_print_help(const char *type, QemuOpts *opts)
-{
-if (user_creatable_print_help(type, opts)) {
-exit(0);
-}
-return true;
-}
-
-
static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, bool list,
Error **errp)
{
@@ -594,7 +576,6 @@ int main(int argc, char **argv)
qcrypto_init(_fatal);
module_call_init(MODULE_INIT_QOM);
-qemu_add_opts(_object_opts);
qemu_add_opts(_trace_opts);
qemu_init_exec_dir(argv[0]);
@@ -747,14 +728,9 @@ int main(int argc, char **argv)
case '?':
error_report("Try `%s --help' for more information.", argv[0]);
exit(EXIT_FAILURE);
-case QEMU_NBD_OPT_OBJECT: {
-QemuOpts *opts;
-opts = qemu_opts_parse_noisily(_object_opts,
- optarg, true);
-if (!opts) {
-exit(EXIT_FAILURE);
-}
-} break;
+case QEMU_NBD_OPT_OBJECT:
+user_creatable_process_cmdline(optarg);
+break;
case QEMU_NBD_OPT_TLSCREDS:
tlscredsid = optarg;
break;
@@ -802,10 +778,6 @@ int main(int argc, char **argv)
export_name = "";
}
-qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- qemu_nbd_object_print_help, _fatal);
-
if (!trace_init_backends()) {
exit(1);
}
--
2.29.2
[PATCH v2 18/31] qapi/qom: Add ObjectOptions for x-remote-object
This adds a QAPI schema for the properties of the x-remote-object
object.
Signed-off-by: Kevin Wolf
---
qapi/qom.json | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index f8ff322df0..6793342e81 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -641,6 +641,20 @@
{ 'struct': 'PrManagerHelperProperties',
'data': { 'path': 'str' } }
+##
+# @RemoteObjectProperties:
+#
+# Properties for x-remote-object objects.
+#
+# @fd: file descriptor name previously passed via 'getfd' command
+#
+# @devid: the id of the device to be associated with the file descriptor
+#
+# Since: 6.0
+##
+{ 'struct': 'RemoteObjectProperties',
+ 'data': { 'fd': 'str', 'devid': 'str' } }
+
##
# @RngProperties:
#
@@ -762,7 +776,8 @@
'tls-creds-anon',
'tls-creds-psk',
'tls-creds-x509',
-'tls-cipher-suites'
+'tls-cipher-suites',
+'x-remote-object'
] }
##
@@ -815,7 +830,8 @@
'tls-creds-anon': 'TlsCredsAnonProperties',
'tls-creds-psk': 'TlsCredsPskProperties',
'tls-creds-x509': 'TlsCredsX509Properties',
- 'tls-cipher-suites': 'TlsCredsProperties'
+ 'tls-cipher-suites': 'TlsCredsProperties',
+ 'x-remote-object':'RemoteObjectProperties'
} }
##
--
2.29.2
[PATCH v2 05/31] qapi/qom: Add ObjectOptions for cryptodev-*
This adds a QAPI schema for the properties of the cryptodev-* objects.
These interfaces have some questionable aspects (cryptodev-backend is
really an abstract base class without function, and the queues option
only makes sense for cryptodev-vhost-user), but as the goal is to
represent the existing interface in QAPI, leave these things in place.
Signed-off-by: Kevin Wolf
---
qapi/qom.json | 34 ++
1 file changed, 34 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 30ed179bc1..1dbc95fb53 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -204,6 +204,34 @@
'returns': [ 'ObjectPropertyInfo' ],
'allow-preconfig': true }
+##
+# @CryptodevBackendProperties:
+#
+# Properties for cryptodev-backend and cryptodev-backend-builtin objects.
+#
+# @queues: the number of queues for the cryptodev backend. Ignored for
+# cryptodev-backend and must be 1 for cryptodev-backend-builtin.
+# (default: 1)
+#
+# Since: 2.8
+##
+{ 'struct': 'CryptodevBackendProperties',
+ 'data': { '*queues': 'uint32' } }
+
+##
+# @CryptodevVhostUserProperties:
+#
+# Properties for cryptodev-vhost-user objects.
+#
+# @chardev: the name of a unix domain socket character device that connects to
+# the vhost-user server
+#
+# Since: 2.12
+##
+{ 'struct': 'CryptodevVhostUserProperties',
+ 'base': 'CryptodevBackendProperties',
+ 'data': { 'chardev': 'str' } }
+
##
# @IothreadProperties:
#
@@ -239,6 +267,9 @@
'authz-listfile',
'authz-pam',
'authz-simple',
+'cryptodev-backend',
+'cryptodev-backend-builtin',
+'cryptodev-vhost-user',
'iothread'
] }
@@ -262,6 +293,9 @@
'authz-listfile': 'AuthZListFileProperties',
'authz-pam': 'AuthZPAMProperties',
'authz-simple': 'AuthZSimpleProperties',
+ 'cryptodev-backend': 'CryptodevBackendProperties',
+ 'cryptodev-backend-builtin': 'CryptodevBackendProperties',
+ 'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
'iothread': 'IothreadProperties'
} }
--
2.29.2
[PATCH v2 20/31] qom: Make "object" QemuOptsList optional
This code is going away anyway, but for a few more commits, we'll be in
a state where some binaries still use QemuOpts and others don't. If the
"object" QemuOptsList doesn't even exist, we don't have to remove (or
fail to remove, and therefore abort) a user creatable object from it.
Signed-off-by: Kevin Wolf
---
qom/object_interfaces.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 1e9ad6f08a..b9a99c8bf4 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -290,6 +290,7 @@ void user_creatable_print_help_from_qdict(QDict *args)
bool user_creatable_del(const char *id, Error **errp)
{
+QemuOptsList *opts_list;
Object *container;
Object *obj;
@@ -309,8 +310,10 @@ bool user_creatable_del(const char *id, Error **errp)
* if object was defined on the command-line, remove its corresponding
* option group entry
*/
-qemu_opts_del(qemu_opts_find(qemu_find_opts_err("object", _abort),
- id));
+opts_list = qemu_find_opts_err("object", NULL);
+if (opts_list) {
+qemu_opts_del(qemu_opts_find(opts_list, id));
+}
object_unparent(obj);
return true;
--
2.29.2
[PATCH v2 14/31] qapi/qom: Add ObjectOptions for filter-*
This adds a QAPI schema for the properties of the filter-* objects.
Some parts of the interface (in particular NetfilterProperties.position)
are very unusual for QAPI, but for now just describe the existing
interface.
net.json can't be included in qom.json because the storage daemon
doesn't have it. NetFilterDirection is still required in the new object
property definitions in qom.json, so move this enum to common.json.
Signed-off-by: Kevin Wolf
---
qapi/common.json | 20 +++
qapi/net.json| 20 ---
qapi/qom.json| 143 +++
3 files changed, 163 insertions(+), 20 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index 2dad4fadc3..b87e7f9039 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -165,3 +165,23 @@
##
{ 'enum': 'HostMemPolicy',
'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
+
+##
+# @NetFilterDirection:
+#
+# Indicates whether a netfilter is attached to a netdev's transmit queue or
+# receive queue or both.
+#
+# @all: the filter is attached both to the receive and the transmit
+# queue of the netdev (default).
+#
+# @rx: the filter is attached to the receive queue of the netdev,
+# where it will receive packets sent to the netdev.
+#
+# @tx: the filter is attached to the transmit queue of the netdev,
+# where it will receive packets sent by the netdev.
+#
+# Since: 2.5
+##
+{ 'enum': 'NetFilterDirection',
+ 'data': [ 'all', 'rx', 'tx' ] }
diff --git a/qapi/net.json b/qapi/net.json
index c31748c87f..af3f5b0fda 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -492,26 +492,6 @@
'vhost-user': 'NetdevVhostUserOptions',
'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
-##
-# @NetFilterDirection:
-#
-# Indicates whether a netfilter is attached to a netdev's transmit queue or
-# receive queue or both.
-#
-# @all: the filter is attached both to the receive and the transmit
-# queue of the netdev (default).
-#
-# @rx: the filter is attached to the receive queue of the netdev,
-# where it will receive packets sent to the netdev.
-#
-# @tx: the filter is attached to the transmit queue of the netdev,
-# where it will receive packets sent by the netdev.
-#
-# Since: 2.5
-##
-{ 'enum': 'NetFilterDirection',
- 'data': [ 'all', 'rx', 'tx' ] }
-
##
# @RxState:
#
diff --git a/qapi/qom.json b/qapi/qom.json
index 8e4414f843..e3357f5123 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -313,6 +313,137 @@
'data': { 'addr': 'str' ,
'*id-list': 'str' } }
+##
+# @NetfilterInsert:
+#
+# Indicates where to insert a netfilter relative to a given other filter.
+#
+# @before: insert before the specified filter
+#
+# @behind: insert behind the specified filter
+#
+# Since: 5.0
+##
+{ 'enum': 'NetfilterInsert',
+ 'data': [ 'before', 'behind' ] }
+
+##
+# @NetfilterProperties:
+#
+# Properties for objects of classes derived from netfilter.
+#
+# @netdev: id of the network device backend to filter
+#
+# @queue: indicates which queue(s) to filter (default: all)
+#
+# @status: indicates whether the filter is enabled ("on") or disabled ("off")
+# (default: "on")
+#
+# @position: specifies where the filter should be inserted in the filter list.
+#"head" means the filter is inserted at the head of the filter
list,
+#before any existing filters.
+#"tail" means the filter is inserted at the tail of the filter
list,
+#behind any existing filters (default).
+#"id=" means the filter is inserted before or behind the filter
+#specified by , depending on the @insert property.
+#(default: "tail")
+#
+# @insert: where to insert the filter relative to the filter given in
@position.
+# Ignored if @position is "head" or "tail". (default: behind)
+#
+# Since: 2.5
+##
+{ 'struct': 'NetfilterProperties',
+ 'data': { 'netdev': 'str',
+'*queue': 'NetFilterDirection',
+'*status': 'str',
+'*position': 'str',
+'*insert': 'NetfilterInsert' } }
+
+##
+# @FilterBufferProperties:
+#
+# Properties for filter-buffer objects.
+#
+# @interval: a non-zero interval in microseconds. All packets arriving in the
+#given interval are delayed until the end of the interval.
+#
+# Since: 2.5
+##
+{ 'struct': 'FilterBufferProperties',
+ 'base': 'NetfilterProperties',
+ 'data': { 'interval': 'uint32' } }
+
+##
+# @FilterDumpProperties:
+#
+# Properties for filter-dump objects.
+#
+# @file: the filename where the dumped packets should be stored
+#
+# @maxlen: maximum number of bytes in a packet that are stored (default: 65536)
+#
+# Since: 2.5
+##
+{ 'struct': 'FilterDumpProperties',
+ 'base': 'NetfilterProperties',
+ 'data': { 'file': 'str',
+'*maxlen': 'uint32' } }
+
+##
+# @FilterMirrorProperties:
+#
+# Properties for filter-mirror objects.
+#
+# @outdev: the name of a character device backend to which all incoming packets
+#
[PATCH v2 10/31] qapi/qom: Add ObjectOptions for secret*, deprecate 'loaded'
This adds a QAPI schema for the properties of the secret* objects.
The 'loaded' property doesn't seem to make sense as an external
interface: It is automatically set to true in ucc->complete, and
explicitly setting it to true earlier just means that additional options
will be silently ignored.
In other words, the 'loaded' property is useless. Mark it as deprecated
in the schema from the start.
Signed-off-by: Kevin Wolf
---
qapi/crypto.json | 61 ++
qapi/qom.json | 5
docs/system/deprecated.rst | 11 +++
3 files changed, 77 insertions(+)
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 2aebe6fa20..0fef3de66d 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -381,3 +381,64 @@
'discriminator': 'format',
'data': {
'luks': 'QCryptoBlockAmendOptionsLUKS' } }
+
+##
+# @SecretCommonProperties:
+#
+# Properties for objects of classes derived from secret-common.
+#
+# @loaded: if true, the secret is loaded immediately when applying this option
+# and will probably fail when processing the next option. Don't use;
+# only provided for compatibility. (default: false)
+#
+# @format: the data format that the secret is provided in (default: raw)
+#
+# @keyid: the name of another secret that should be used to decrypt the
+# provided data. If not present, the data is assumed to be unencrypted.
+#
+# @iv: the random initialization vector used for encryption of this particular
+# secret. Should be a base64 encrypted string of the 16-byte IV. Mandatory
+# if @keyid is given. Ignored if @keyid is absent.
+#
+# Features:
+# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense,
+# and false is already the default.
+#
+# Since: 2.6
+##
+{ 'struct': 'SecretCommonProperties',
+ 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
+'*format': 'QCryptoSecretFormat',
+'*keyid': 'str',
+'*iv': 'str' } }
+
+##
+# @SecretProperties:
+#
+# Properties for secret objects.
+#
+# Either @data or @file must be provided, but not both.
+#
+# @data: the associated with the secret from
+#
+# @file: the filename to load the data associated with the secret from
+#
+# Since: 2.6
+##
+{ 'struct': 'SecretProperties',
+ 'base': 'SecretCommonProperties',
+ 'data': { '*data': 'str',
+'*file': 'str' } }
+
+##
+# @SecretKeyringProperties:
+#
+# Properties for secret_keyring objects.
+#
+# @serial: serial number that identifies a key to get from the kernel
+#
+# Since: 5.1
+##
+{ 'struct': 'SecretKeyringProperties',
+ 'base': 'SecretCommonProperties',
+ 'data': { 'serial': 'int32' } }
diff --git a/qapi/qom.json b/qapi/qom.json
index 449dca8ec5..2668ad8369 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -7,6 +7,7 @@
{ 'include': 'authz.json' }
{ 'include': 'block-core.json' }
{ 'include': 'common.json' }
+{ 'include': 'crypto.json' }
##
# = QEMU Object Model (QOM)
@@ -449,6 +450,8 @@
'rng-builtin',
'rng-egd',
'rng-random',
+'secret',
+'secret_keyring',
'throttle-group'
] }
@@ -483,6 +486,8 @@
'rng-builtin':'RngProperties',
'rng-egd':'RngEgdProperties',
'rng-random': 'RngRandomProperties',
+ 'secret': 'SecretProperties',
+ 'secret_keyring': 'SecretKeyringProperties',
'throttle-group': 'ThrottleGroupProperties'
} }
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 79991c2893..78b175cb59 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -155,6 +155,17 @@ other options have been processed. This will either have
no effect (if
``opened`` was the last option) or cause errors. The property is therefore
useless and should not be specified.
+``loaded`` property of ``secret`` and ``secret_keyring`` objects (since 6.0.0)
+''
+
+The only effect of specifying ``loaded=on`` in the command line or QMP
+``object-add`` is that the secret is loaded immediately, possibly before all
+other options have been processed. This will either have no effect (if
+``loaded`` was the last option) or cause options to be effectively ignored as
+if they were not given. The property is therefore useless and should not be
+specified.
+
+
QEMU Machine Protocol (QMP) commands
--
2.29.2
[PATCH v2 08/31] qapi/qom: Add ObjectOptions for rng-*, deprecate 'opened'
This adds a QAPI schema for the properties of the rng-* objects.
The 'opened' property doesn't seem to make sense as an external
interface: It is automatically set to true in ucc->complete, and
explicitly setting it to true earlier just means that trying to set
additional options will result in an error. After the property has once
been set to true (i.e. when the object construction has completed), it
can never be reset to false. In other words, the 'opened' property is
useless. Mark it as deprecated in the schema from the start.
Signed-off-by: Kevin Wolf
---
qapi/qom.json | 56 --
docs/system/deprecated.rst | 9 ++
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index 1a869006a1..73f28f9608 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -380,6 +380,52 @@
'*hugetlbsize': 'size',
'*seal': 'bool' } }
+##
+# @RngProperties:
+#
+# Properties for objects of classes derived from rng.
+#
+# @opened: if true, the device is opened immediately when applying this option
+# and will probably fail when processing the next option. Don't use;
+# only provided for compatibility. (default: false)
+#
+# Features:
+# @deprecated: Member @opened is deprecated. Setting true doesn't make sense,
+# and false is already the default.
+#
+# Since: 1.3
+##
+{ 'struct': 'RngProperties',
+ 'data': { '*opened': { 'type': 'bool', 'features': ['deprecated'] } } }
+
+##
+# @RngEgdProperties:
+#
+# Properties for rng-egd objects.
+#
+# @chardev: the name of a character device backend that provides the connection
+# to the RNG daemon
+#
+# Since: 1.3
+##
+{ 'struct': 'RngEgdProperties',
+ 'base': 'RngProperties',
+ 'data': { 'chardev': 'str' } }
+
+##
+# @RngRandomProperties:
+#
+# Properties for rng-random objects.
+#
+# @filename: the filename of the device on the host to obtain entropy from
+#(default: "/dev/urandom")
+#
+# Since: 1.3
+##
+{ 'struct': 'RngRandomProperties',
+ 'base': 'RngProperties',
+ 'data': { '*filename': 'str' } }
+
##
# @ObjectType:
#
@@ -398,7 +444,10 @@
'iothread',
'memory-backend-file',
'memory-backend-memfd',
-'memory-backend-ram'
+'memory-backend-ram',
+'rng-builtin',
+'rng-egd',
+'rng-random'
] }
##
@@ -428,7 +477,10 @@
'iothread': 'IothreadProperties',
'memory-backend-file':'MemoryBackendFileProperties',
'memory-backend-memfd': 'MemoryBackendMemfdProperties',
- 'memory-backend-ram': 'MemoryBackendProperties'
+ 'memory-backend-ram': 'MemoryBackendProperties',
+ 'rng-builtin':'RngProperties',
+ 'rng-egd':'RngEgdProperties',
+ 'rng-random': 'RngRandomProperties'
} }
##
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 00b694e053..79991c2893 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -146,6 +146,15 @@ library enabled as a cryptography provider.
Neither the ``nettle`` library, or the built-in cryptography provider are
supported on FIPS enabled hosts.
+``opened`` property of ``rng-*`` objects (since 6.0.0)
+''
+
+The only effect of specifying ``opened=on`` in the command line or QMP
+``object-add`` is that the device is opened immediately, possibly before all
+other options have been processed. This will either have no effect (if
+``opened`` was the last option) or cause errors. The property is therefore
+useless and should not be specified.
+
QEMU Machine Protocol (QMP) commands
--
2.29.2
[PATCH v2 03/31] qapi/qom: Add ObjectOptions for iothread
Add an ObjectOptions union that will eventually describe the options of
all user creatable object types. As unions can't exist without any
branches, also add the first object type.
This adds a QAPI schema for the properties of the iothread object.
Signed-off-by: Kevin Wolf
---
qapi/qom.json | 53 +++
1 file changed, 53 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 96c91c1faf..bf2ecb34be 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -202,6 +202,59 @@
'returns': [ 'ObjectPropertyInfo' ],
'allow-preconfig': true }
+##
+# @IothreadProperties:
+#
+# Properties for iothread objects.
+#
+# @poll-max-ns: the maximum number of nanoseconds to busy wait for events.
+# 0 means polling is disabled (default: 32768 on POSIX hosts,
+# 0 otherwise)
+#
+# @poll-grow: the multiplier used to increase the polling time when the
+# algorithm detects it is missing events due to not polling long
+# enough. 0 selects a default behaviour (default: 0)
+#
+# @poll-shrink: the divisor used to decrease the polling time when the
+# algorithm detects it is spending too long polling without
+# encountering events. 0 selects a default behaviour (default: 0)
+#
+# Since: 2.0
+##
+{ 'struct': 'IothreadProperties',
+ 'data': { '*poll-max-ns': 'int',
+'*poll-grow': 'int',
+'*poll-shrink': 'int' } }
+
+##
+# @ObjectType:
+#
+# Since: 6.0
+##
+{ 'enum': 'ObjectType',
+ 'data': [
+'iothread'
+ ] }
+
+##
+# @ObjectOptions:
+#
+# Describes the options of a user creatable QOM object.
+#
+# @qom-type: the class name for the object to be created
+#
+# @id: the name of the new object
+#
+# Since: 6.0
+##
+{ 'union': 'ObjectOptions',
+ 'base': { 'qom-type': 'ObjectType',
+'id': 'str' },
+ 'discriminator': 'qom-type',
+ 'data': {
+ 'iothread': 'IothreadProperties'
+ } }
+
##
# @object-add:
#
--
2.29.2
[PATCH v2 11/31] qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded'
This adds a QAPI schema for the properties of the tls-* objects.
The 'loaded' property doesn't seem to make sense as an external
interface: It is automatically set to true in ucc->complete, and
explicitly setting it to true earlier just means that additional options
will be silently ignored.
In other words, the 'loaded' property is useless. Mark it as deprecated
in the schema from the start.
Signed-off-by: Kevin Wolf
---
qapi/crypto.json | 98
qapi/qom.json| 12 +-
2 files changed, 108 insertions(+), 2 deletions(-)
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 0fef3de66d..7116ae9a46 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -442,3 +442,101 @@
{ 'struct': 'SecretKeyringProperties',
'base': 'SecretCommonProperties',
'data': { 'serial': 'int32' } }
+
+##
+# @TlsCredsProperties:
+#
+# Properties for objects of classes derived from tls-creds.
+#
+# @verify-peer: if true the peer credentials will be verified once the
+# handshake is completed. This is a no-op for anonymous
+# credentials. (default: true)
+#
+# @dir: the path of the directory that contains the credential files
+#
+# @endpoint: whether the QEMU network backend that uses the credentials will be
+#acting as a client or as a server (default: client)
+#
+# @priority: a gnutls priority string as described at
+#https://gnutls.org/manual/html_node/Priority-Strings.html
+#
+# Since: 2.5
+##
+{ 'struct': 'TlsCredsProperties',
+ 'data': { '*verify-peer': 'bool',
+'*dir': 'str',
+'*endpoint': 'QCryptoTLSCredsEndpoint',
+'*priority': 'str' } }
+
+##
+# @TlsCredsAnonProperties:
+#
+# Properties for tls-creds-anon objects.
+#
+# @loaded: if true, the credentials are loaded immediately when applying this
+# option and will ignore options that are processed later. Don't use;
+# only provided for compatibility. (default: false)
+#
+# Features:
+# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense,
+# and false is already the default.
+#
+# Since: 2.5
+##
+{ 'struct': 'TlsCredsAnonProperties',
+ 'base': 'TlsCredsProperties',
+ 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] } } }
+
+##
+# @TlsCredsPskProperties:
+#
+# Properties for tls-creds-psk objects.
+#
+# @loaded: if true, the credentials are loaded immediately when applying this
+# option and will ignore options that are processed later. Don't use;
+# only provided for compatibility. (default: false)
+#
+# @username: the username which will be sent to the server. For clients only.
+#If absent, "qemu" is sent and the property will read back as an
+#empty string.
+#
+# Features:
+# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense,
+# and false is already the default.
+#
+# Since: 3.0
+##
+{ 'struct': 'TlsCredsPskProperties',
+ 'base': 'TlsCredsProperties',
+ 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
+'*username': 'str' } }
+
+##
+# @TlsCredsX509Properties:
+#
+# Properties for tls-creds-x509 objects.
+#
+# @loaded: if true, the credentials are loaded immediately when applying this
+# option and will ignore options that are processed later. Don't use;
+# only provided for compatibility. (default: false)
+#
+# @sanity-check: if true, perform some sanity checks before using the
+#credentials (default: true)
+#
+# @passwordid: For the server-key.pem and client-key.pem files which contain
+# sensitive private keys, it is possible to use an encrypted
+# version by providing the @passwordid parameter. This provides
+# the ID of a previously created secret object containing the
+# password for decryption.
+#
+# Features:
+# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense,
+# and false is already the default.
+#
+# Since: 2.5
+##
+{ 'struct': 'TlsCredsX509Properties',
+ 'base': 'TlsCredsProperties',
+ 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
+'*sanity-check': 'bool',
+'*passwordid': 'str' } }
diff --git a/qapi/qom.json b/qapi/qom.json
index 2668ad8369..f22b7aa99b 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -452,7 +452,11 @@
'rng-random',
'secret',
'secret_keyring',
-'throttle-group'
+'throttle-group',
+'tls-creds-anon',
+'tls-creds-psk',
+'tls-creds-x509',
+'tls-cipher-suites'
] }
##
@@ -488,7 +492,11 @@
'rng-random': 'RngRandomProperties',
'secret': 'SecretProperties',
'secret_keyring': 'SecretKeyringProperties',
- 'throttle-group': 'ThrottleGroupProperties'
+ 'throttle-group': 'ThrottleGroupProperties',
+
[PATCH v2 13/31] qapi/qom: Add ObjectOptions for colo-compare
This adds a QAPI schema for the properties of the colo-compare object.
Signed-off-by: Kevin Wolf
---
qapi/qom.json | 49 +
1 file changed, 49 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index 4b1cd4b8dc..8e4414f843 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -222,6 +222,53 @@
'data': { 'if': 'str',
'canbus': 'str' } }
+##
+# @ColoCompareProperties:
+#
+# Properties for colo-compare objects.
+#
+# @primary_in: name of the character device backend to use for the primary
+# input (incoming packets are redirected to @outdev)
+#
+# @secondary_in: name of the character device backend to use for secondary
+#input (incoming packets are only compared to the input on
+#@primary_in and then dropped)
+#
+# @outdev: name of the character device backend to use for output
+#
+# @iothread: name of the iothread to run in
+#
+# @notify_dev: name of the character device backend to be used to communicate
+# with the remote colo-frame (only for Xen COLO)
+#
+# @compare_timeout: the maximum time to hold a packet from @primary_in for
+# comparison with an incoming packet on @secondary_in in
+# milliseconds (default: 3000)
+#
+# @expired_scan_cycle: the interval at which colo-compare checks whether
+# packets from @primary have timed out, in milliseconds
+# (default: 3000)
+#
+# @max_queue_size: the maximum number of packets to keep in the queue for
+# comparing with incoming packets from @secondary_in. If the
+# queue is full and addtional packets are received, the
+# addtional packets are dropped. (default: 1024)
+#
+# @vnet_hdr_support: if true, vnet header support is enabled (default: false)
+#
+# Since: 2.8
+##
+{ 'struct': 'ColoCompareProperties',
+ 'data': { 'primary_in': 'str',
+'secondary_in': 'str',
+'outdev': 'str',
+'iothread': 'str',
+'*notify_dev': 'str',
+'*compare_timeout': 'uint64',
+'*expired_scan_cycle': 'uint32',
+'*max_queue_size': 'uint32',
+'*vnet_hdr_support': 'bool' } }
+
##
# @CryptodevBackendProperties:
#
@@ -456,6 +503,7 @@
'authz-simple',
'can-bus',
'can-host-socketcan',
+'colo-compare',
'cryptodev-backend',
'cryptodev-backend-builtin',
'cryptodev-vhost-user',
@@ -497,6 +545,7 @@
'authz-pam': 'AuthZPAMProperties',
'authz-simple': 'AuthZSimpleProperties',
'can-host-socketcan': 'CanHostSocketcanProperties',
+ 'colo-compare': 'ColoCompareProperties',
'cryptodev-backend': 'CryptodevBackendProperties',
'cryptodev-backend-builtin': 'CryptodevBackendProperties',
'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
--
2.29.2
[PATCH v2 12/31] qapi/qom: Add ObjectOptions for can-*
This adds a QAPI schema for the properties of the can-* objects.
can-bus doesn't have any properties, so it only needs to be added to the
ObjectType enum without adding a new branch to ObjectOptions.
Signed-off-by: Kevin Wolf
---
qapi/qom.json | 18 ++
1 file changed, 18 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index f22b7aa99b..4b1cd4b8dc 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -207,6 +207,21 @@
'returns': [ 'ObjectPropertyInfo' ],
'allow-preconfig': true }
+##
+# @CanHostSocketcanProperties:
+#
+# Properties for can-host-socketcan objects.
+#
+# @if: interface name of the host system CAN bus to connect to
+#
+# @canbus: object ID of the can-bus object to connect to the host interface
+#
+# Since: 2.12
+##
+{ 'struct': 'CanHostSocketcanProperties',
+ 'data': { 'if': 'str',
+'canbus': 'str' } }
+
##
# @CryptodevBackendProperties:
#
@@ -439,6 +454,8 @@
'authz-listfile',
'authz-pam',
'authz-simple',
+'can-bus',
+'can-host-socketcan',
'cryptodev-backend',
'cryptodev-backend-builtin',
'cryptodev-vhost-user',
@@ -479,6 +496,7 @@
'authz-listfile': 'AuthZListFileProperties',
'authz-pam': 'AuthZPAMProperties',
'authz-simple': 'AuthZSimpleProperties',
+ 'can-host-socketcan': 'CanHostSocketcanProperties',
'cryptodev-backend': 'CryptodevBackendProperties',
'cryptodev-backend-builtin': 'CryptodevBackendProperties',
'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
--
2.29.2
[PATCH v2 17/31] qapi/qom: Add ObjectOptions for input-*
This adds a QAPI schema for the properties of the input-* objects.
ui.json cannot be included in qom.json because the storage daemon can't
use it, so move GrabToggleKeys to common.json.
Signed-off-by: Kevin Wolf
---
qapi/common.json | 12 ++
qapi/qom.json| 58
qapi/ui.json | 13 +--
3 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index b87e7f9039..7c976296f0 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -185,3 +185,15 @@
##
{ 'enum': 'NetFilterDirection',
'data': [ 'all', 'rx', 'tx' ] }
+
+##
+# @GrabToggleKeys:
+#
+# Keys to toggle input-linux between host and guest.
+#
+# Since: 4.0
+#
+##
+{ 'enum': 'GrabToggleKeys',
+ 'data': [ 'ctrl-ctrl', 'alt-alt', 'shift-shift','meta-meta', 'scrolllock',
+'ctrl-scrolllock' ] }
diff --git a/qapi/qom.json b/qapi/qom.json
index d5f68b5c89..f8ff322df0 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -444,6 +444,60 @@
'base': 'NetfilterProperties',
'data': { '*vnet_hdr_support': 'bool' } }
+##
+# @InputBarrierProperties:
+#
+# Properties for input-barrier objects.
+#
+# @name: the screen name as declared in the screens section of barrier.conf
+#
+# @server: hostname of the Barrier server (default: "localhost")
+#
+# @port: TCP port of the Barrier server (default: "24800")
+#
+# @x-origin: x coordinate of the leftmost pixel on the guest screen
+#(default: "0")
+#
+# @y-origin: y coordinate of he topmost pixel on the guest screen (default:
"0")
+#
+# @width: the width of secondary screen in pixels (default: "1920")
+#
+# @height: the height of secondary screen in pixels (default: "1080")
+#
+# Since: 4.2
+##
+{ 'struct': 'InputBarrierProperties',
+ 'data': { 'name': 'str',
+'*server': 'str',
+'*port': 'str',
+'*x-origin': 'str',
+'*y-origin': 'str',
+'*width': 'str',
+'*height': 'str' } }
+
+##
+# @InputLinuxProperties:
+#
+# Properties for input-linux objects.
+#
+# @evdev: the path of the host evdev device to use
+#
+# @grab_all: if true, grab is toggled for all devices (e.g. both keyboard and
+#mouse) instead of just one device (default: false)
+#
+# @repeat: enables auto-repeat events (default: false)
+#
+# @grab-toggle: the key or key combination that toggles device grab
+# (default: ctrl-ctrl)
+#
+# Since: 2.6
+##
+{ 'struct': 'InputLinuxProperties',
+ 'data': { 'evdev': 'str',
+'*grab_all': 'bool',
+'*repeat': 'bool',
+'*grab-toggle': 'GrabToggleKeys' } }
+
##
# @IothreadProperties:
#
@@ -689,6 +743,8 @@
'filter-redirector',
'filter-replay',
'filter-rewriter',
+'input-barrier',
+'input-linux',
'iothread',
'memory-backend-file',
'memory-backend-memfd',
@@ -741,6 +797,8 @@
'filter-redirector': 'FilterRedirectorProperties',
'filter-replay': 'NetfilterProperties',
'filter-rewriter':'FilterRewriterProperties',
+ 'input-barrier': 'InputBarrierProperties',
+ 'input-linux':'InputLinuxProperties',
'iothread': 'IothreadProperties',
'memory-backend-file':'MemoryBackendFileProperties',
'memory-backend-memfd': 'MemoryBackendMemfdProperties',
diff --git a/qapi/ui.json b/qapi/ui.json
index d08d72b439..cc1882108b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -6,6 +6,7 @@
# = Remote desktop
##
+{ 'include': 'common.json' }
{ 'include': 'sockets.json' }
##
@@ -1021,18 +1022,6 @@
'*head' : 'int',
'events' : [ 'InputEvent' ] } }
-##
-# @GrabToggleKeys:
-#
-# Keys to toggle input-linux between host and guest.
-#
-# Since: 4.0
-#
-##
-{ 'enum': 'GrabToggleKeys',
- 'data': [ 'ctrl-ctrl', 'alt-alt', 'shift-shift','meta-meta', 'scrolllock',
-'ctrl-scrolllock' ] }
-
##
# @DisplayGTK:
#
--
2.29.2
[PATCH v2 16/31] qapi/qom: Add ObjectOptions for confidential-guest-support
This adds a QAPI schema for the properties of the objects implementing
the confidential-guest-support interface.
pef-guest and s390x-pv-guest don't have any properties, so they only
need to be added to the ObjectType enum without adding a new branch to
ObjectOptions.
Signed-off-by: Kevin Wolf
---
qapi/qom.json | 37 +
1 file changed, 37 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index e7184122e9..d5f68b5c89 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -633,6 +633,38 @@
'base': 'RngProperties',
'data': { '*filename': 'str' } }
+##
+# @SevGuestProperties:
+#
+# Properties for sev-guest objects.
+#
+# @sev-device: SEV device to use (default: "/dev/sev")
+#
+# @dh-cert-file: guest owners DH certificate (encoded with base64)
+#
+# @session-file: guest owners session parameters (encoded with base64)
+#
+# @policy: SEV policy value (default: 0x1)
+#
+# @handle: SEV firmware handle (default: 0)
+#
+# @cbitpos: C-bit location in page table entry (default: 0)
+#
+# @reduced-phys-bits: number of bits in physical addresses that become
+# unavailable when SEV is enabled
+#
+# Since: 2.12
+##
+{ 'struct': 'SevGuestProperties',
+ 'data': { '*sev-device': 'str',
+'*dh-cert-file': 'str',
+'*session-file': 'str',
+'*policy': 'uint32',
+'*handle': 'uint32',
+'*cbitpos': 'uint32',
+'reduced-phys-bits': 'uint32' },
+ 'if': 'defined(CONFIG_SEV)' }
+
##
# @ObjectType:
#
@@ -661,12 +693,15 @@
'memory-backend-file',
'memory-backend-memfd',
'memory-backend-ram',
+{'name': 'pef-guest', 'if': 'defined(CONFIG_PSERIES)' },
'pr-manager-helper',
'rng-builtin',
'rng-egd',
'rng-random',
'secret',
'secret_keyring',
+{'name': 'sev-guest', 'if': 'defined(CONFIG_SEV)' },
+'s390-pv-guest',
'throttle-group',
'tls-creds-anon',
'tls-creds-psk',
@@ -716,6 +751,8 @@
'rng-random': 'RngRandomProperties',
'secret': 'SecretProperties',
'secret_keyring': 'SecretKeyringProperties',
+ 'sev-guest': { 'type': 'SevGuestProperties',
+ 'if': 'defined(CONFIG_SEV)' },
'throttle-group': 'ThrottleGroupProperties',
'tls-creds-anon': 'TlsCredsAnonProperties',
'tls-creds-psk': 'TlsCredsPskProperties',
--
2.29.2
