hw/block/xen-block.c | 3 ++- hw/char/xen_console.c | 14 ++++++++------ hw/xen/trace-events | 1 + hw/xen/xen-bus-helper.c | 22 ++++++++++++++++++++++ hw/xen/xen-bus.c | 14 ++++++++++++-- include/hw/xen/xen-bus-helper.h | 4 ++++ include/hw/xen/xen-bus.h | 1 + 7 files changed, 50 insertions(+), 9 deletions(-)
Hello, First patch from David introduces a new helper to fetch xenstore nodes, while second patch removes the usage of scanf related functions with the "%ms" format specifier, as it's not supported by the FreeBSD scanf libc implementation. Thanks, Roger. David Woodhouse (1): hw/xen: Add xs_node_read() helper function Roger Pau Monné (1): xen: do not use '%ms' scanf specifier hw/block/xen-block.c | 3 ++- hw/char/xen_console.c | 14 ++++++++------ hw/xen/trace-events | 1 + hw/xen/xen-bus-helper.c | 22 ++++++++++++++++++++++ hw/xen/xen-bus.c | 14 ++++++++++++-- include/hw/xen/xen-bus-helper.h | 4 ++++ include/hw/xen/xen-bus.h | 1 + 7 files changed, 50 insertions(+), 9 deletions(-) -- 2.46.0
On Fri, 2025-01-10 at 10:35 +0100, Roger Pau Monne wrote:
> Hello,
>
> First patch from David introduces a new helper to fetch xenstore nodes,
> while second patch removes the usage of scanf related functions with the
> "%ms" format specifier, as it's not supported by the FreeBSD scanf libc
> implementation.
>
> Thanks, Roger.
Thanks. I've got a handful of non-bugfix cleanups to use the new
xs_node_read in my tree at
https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xs_node_read
David Woodhouse (4):
hw/xen: Use xs_node_read() from xs_node_vscanf()
hw/xen: Use xs_node_read() from xen_console_get_name()
hw/xen: Use xs_node_read() from xen_netdev_get_name()
hw/xen: Use xs_node_read() from xenstore_read_str() instead of open-coding it
I'm slightly dubious about the last one; xen_pvdev.c didn't previously
use anything from xen-bus-helper.c and even hardcodes zero for
XBT_NULL. And I look at the way it deliberately reallocates the string,
and wonder if we should be doing that in qemu_xen_xs_read() for the
true Xen case. And does it even matter anywhere except Windows?
On Fri, Jan 10, 2025 at 10:02:53AM +0000, David Woodhouse wrote: > On Fri, 2025-01-10 at 10:35 +0100, Roger Pau Monne wrote: > > Hello, > > > > First patch from David introduces a new helper to fetch xenstore nodes, > > while second patch removes the usage of scanf related functions with the > > "%ms" format specifier, as it's not supported by the FreeBSD scanf libc > > implementation. > > > > Thanks, Roger. > > Thanks. I've got a handful of non-bugfix cleanups to use the new > xs_node_read in my tree at > https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xs_node_read > > David Woodhouse (4): > hw/xen: Use xs_node_read() from xs_node_vscanf() > hw/xen: Use xs_node_read() from xen_console_get_name() > hw/xen: Use xs_node_read() from xen_netdev_get_name() > hw/xen: Use xs_node_read() from xenstore_read_str() instead of open-coding it Acked-by: Roger Pau Monné <roger.pau@citrix.com> > I'm slightly dubious about the last one; xen_pvdev.c didn't previously > use anything from xen-bus-helper.c and even hardcodes zero for > XBT_NULL. And I look at the way it deliberately reallocates the string, > and wonder if we should be doing that in qemu_xen_xs_read() for the > true Xen case. And does it even matter anywhere except Windows? I would take the opportunity to use XBT_NULL instead of 0 on xen_pvdev.c for the transaction. Thanks, Roger.
On Wed, 2025-01-15 at 15:34 +0100, Roger Pau Monné wrote: > On Fri, Jan 10, 2025 at 10:02:53AM +0000, David Woodhouse wrote: > > On Fri, 2025-01-10 at 10:35 +0100, Roger Pau Monne wrote: > > > Hello, > > > > > > First patch from David introduces a new helper to fetch xenstore nodes, > > > while second patch removes the usage of scanf related functions with the > > > "%ms" format specifier, as it's not supported by the FreeBSD scanf libc > > > implementation. > > > > > > Thanks, Roger. > > > > Thanks. I've got a handful of non-bugfix cleanups to use the new > > xs_node_read in my tree at > > https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xs_node_read > > > > David Woodhouse (4): > > hw/xen: Use xs_node_read() from xs_node_vscanf() > > hw/xen: Use xs_node_read() from xen_console_get_name() > > hw/xen: Use xs_node_read() from xen_netdev_get_name() > > hw/xen: Use xs_node_read() from xenstore_read_str() instead of open-coding it > > Acked-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. I'll add that on each of the four. > > I'm slightly dubious about the last one; xen_pvdev.c didn't previously > > use anything from xen-bus-helper.c and even hardcodes zero for > > XBT_NULL. And I look at the way it deliberately reallocates the string, > > and wonder if we should be doing that in qemu_xen_xs_read() for the > > true Xen case. And does it even matter anywhere except Windows? > > I would take the opportunity to use XBT_NULL instead of 0 on > xen_pvdev.c for the transaction. Yeah, probably a separate patch.
From: David Woodhouse <dwmw@amazon.co.uk>
Reduce some duplication.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/xen/trace-events | 1 -
hw/xen/xen-bus-helper.c | 15 ++++++---------
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index 461dee7b23..b67942d07b 100644
--- a/hw/xen/trace-events
+++ b/hw/xen/trace-events
@@ -38,7 +38,6 @@ xen_device_remove_watch(const char *type, char *name, const char *node, const ch
xs_node_create(const char *node) "%s"
xs_node_destroy(const char *node) "%s"
xs_node_vprintf(char *path, char *value) "%s %s"
-xs_node_vscanf(char *path, char *value) "%s %s"
xs_node_read(const char *path, const char *value) "%s %s"
xs_node_watch(char *path) "%s"
xs_node_unwatch(char *path) "%s"
diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
index 0fba7946c5..57697799f3 100644
--- a/hw/xen/xen-bus-helper.c
+++ b/hw/xen/xen-bus-helper.c
@@ -105,25 +105,22 @@ int xs_node_vscanf(struct qemu_xs_handle *h, xs_transaction_t tid,
const char *node, const char *key, Error **errp,
const char *fmt, va_list ap)
{
- char *path, *value;
+ char *value;
int rc;
- path = (strlen(node) != 0) ? g_strdup_printf("%s/%s", node, key) :
- g_strdup(key);
- value = qemu_xen_xs_read(h, tid, path, NULL);
-
- trace_xs_node_vscanf(path, value);
+ if (node && strlen(node) != 0) {
+ value = xs_node_read(h, tid, NULL, errp, "%s/%s", node, key);
+ } else {
+ value = xs_node_read(h, tid, NULL, errp, "%s", key);
+ }
if (value) {
rc = vsscanf(value, fmt, ap);
} else {
- error_setg_errno(errp, errno, "failed to read from '%s'",
- path);
rc = EOF;
}
free(value);
- g_free(path);
return rc;
}
--
2.47.0On Fri, Jan 10, 2025 at 10:03:23AM +0000, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Reduce some duplication. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Thanks, -- Anthony PERARD
From: David Woodhouse <dwmw@amazon.co.uk>
Now that xs_node_read() can construct a node path, no need to open-code it.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/char/xen_console.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 989e75fef8..9338e00473 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -367,28 +367,28 @@ static char *xen_console_get_name(XenDevice *xendev, Error **errp)
if (con->dev == -1) {
XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
- char fe_path[XENSTORE_ABS_PATH_MAX + 1];
int idx = (xen_mode == XEN_EMULATE) ? 0 : 1;
+ Error *local_err = NULL;
char *value;
/* Theoretically we could go up to INT_MAX here but that's overkill */
while (idx < 100) {
if (!idx) {
- snprintf(fe_path, sizeof(fe_path),
- "/local/domain/%u/console", xendev->frontend_id);
+ value = xs_node_read(xenbus->xsh, XBT_NULL, NULL, &local_err,
+ "/local/domain/%u/console",
+ xendev->frontend_id);
} else {
- snprintf(fe_path, sizeof(fe_path),
- "/local/domain/%u/device/console/%u",
- xendev->frontend_id, idx);
+ value = xs_node_read(xenbus->xsh, XBT_NULL, NULL, &local_err,
+ "/local/domain/%u/device/console/%u",
+ xendev->frontend_id, idx);
}
- value = qemu_xen_xs_read(xenbus->xsh, XBT_NULL, fe_path, NULL);
if (!value) {
if (errno == ENOENT) {
con->dev = idx;
+ error_free(local_err);
goto found;
}
- error_setg(errp, "cannot read %s: %s", fe_path,
- strerror(errno));
+ error_propagate(errp, local_err);
return NULL;
}
free(value);
--
2.47.0On Fri, Jan 10, 2025 at 10:03:24AM +0000, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Now that xs_node_read() can construct a node path, no need to open-code it. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Thanks, -- Anthony PERARD
From: David Woodhouse <dwmw@amazon.co.uk>
Now that xs_node_read() can construct a node path, no need to open-code it.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/net/xen_nic.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 97ebd9fa30..5410039490 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -510,23 +510,22 @@ static char *xen_netdev_get_name(XenDevice *xendev, Error **errp)
if (netdev->dev == -1) {
XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
- char fe_path[XENSTORE_ABS_PATH_MAX + 1];
int idx = (xen_mode == XEN_EMULATE) ? 0 : 1;
+ Error *local_err = NULL;
char *value;
/* Theoretically we could go up to INT_MAX here but that's overkill */
while (idx < 100) {
- snprintf(fe_path, sizeof(fe_path),
- "/local/domain/%u/device/vif/%u",
- xendev->frontend_id, idx);
- value = qemu_xen_xs_read(xenbus->xsh, XBT_NULL, fe_path, NULL);
+ value = xs_node_read(xenbus->xsh, XBT_NULL, NULL, &local_err,
+ "/local/domain/%u/device/vif/%u",
+ xendev->frontend_id, idx);
if (!value) {
if (errno == ENOENT) {
netdev->dev = idx;
+ error_free(local_err);
goto found;
}
- error_setg(errp, "cannot read %s: %s", fe_path,
- strerror(errno));
+ error_propagate(errp, local_err);
return NULL;
}
free(value);
--
2.47.0On Fri, Jan 10, 2025 at 10:03:25AM +0000, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Now that xs_node_read() can construct a node path, no need to open-code it. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Thanks, -- Anthony PERARD
From: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/xen/xen_pvdev.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c5ad71e8dc..c9143ba259 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -22,6 +22,7 @@
#include "qemu/main-loop.h"
#include "hw/qdev-core.h"
#include "hw/xen/xen-legacy-backend.h"
+#include "hw/xen/xen-bus-helper.h"
#include "hw/xen/xen_pvdev.h"
/* private */
@@ -81,12 +82,9 @@ int xenstore_write_str(const char *base, const char *node, const char *val)
char *xenstore_read_str(const char *base, const char *node)
{
- char abspath[XEN_BUFSIZE];
- unsigned int len;
char *str, *ret = NULL;
- snprintf(abspath, sizeof(abspath), "%s/%s", base, node);
- str = qemu_xen_xs_read(xenstore, 0, abspath, &len);
+ str = xs_node_read(xenstore, 0, NULL, NULL, "%s/%s", base, node);
if (str != NULL) {
/* move to qemu-allocated memory to make sure
* callers can safely g_free() stuff. */
--
2.47.0On Fri, Jan 10, 2025 at 10:03:26AM +0000, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Thanks, -- Anthony PERARD
© 2016 - 2025 Red Hat, Inc.