Re: [PATCH v2 03/15] net/vhost-vdpa: Fix device compatibility check

2021-10-13 Thread Jason Wang



在 2021/10/8 下午9:34, Kevin Wolf 写道:

vhost-vdpa works only with specific devices. At startup, it second
guesses what the command line option handling will do and error out if
it thinks a non-virtio device will attach to them.

This second guessing is not only ugly, it can lead to wrong error
messages ('-device floppy,netdev=foo' should complain about an unknown
property, not about the wrong kind of network device being attached) and
completely ignores hotplugging.

Drop the old checks and implement .check_peer_type() instead to fix
this. As a nice side effect, it also removes one more dependency on the
legacy QemuOpts infrastructure and even reduces the code size.

Signed-off-by: Kevin Wolf 



Acked-by: Jason Wang 



---
  net/vhost-vdpa.c | 37 ++---
  1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 912686457c..6dc68d8677 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -147,12 +147,26 @@ static bool vhost_vdpa_has_ufo(NetClientState *nc)
  
  }
  
+static bool vhost_vdpa_check_peer_type(NetClientState *nc, ObjectClass *oc,

+   Error **errp)
+{
+const char *driver = object_class_get_name(oc);
+
+if (!g_str_has_prefix(driver, "virtio-net-")) {
+error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
+return false;
+}
+
+return true;
+}
+
  static NetClientInfo net_vhost_vdpa_info = {
  .type = NET_CLIENT_DRIVER_VHOST_VDPA,
  .size = sizeof(VhostVDPAState),
  .cleanup = vhost_vdpa_cleanup,
  .has_vnet_hdr = vhost_vdpa_has_vnet_hdr,
  .has_ufo = vhost_vdpa_has_ufo,
+.check_peer_type = vhost_vdpa_check_peer_type,
  };
  
  static int net_vhost_vdpa_init(NetClientState *peer, const char *device,

@@ -179,24 +193,6 @@ static int net_vhost_vdpa_init(NetClientState *peer, const 
char *device,
  return ret;
  }
  
-static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)

-{
-const char *name = opaque;
-const char *driver, *netdev;
-
-driver = qemu_opt_get(opts, "driver");
-netdev = qemu_opt_get(opts, "netdev");
-if (!driver || !netdev) {
-return 0;
-}
-if (strcmp(netdev, name) == 0 &&
-!g_str_has_prefix(driver, "virtio-net-")) {
-error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
-return -1;
-}
-return 0;
-}
-
  int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
  NetClientState *peer, Error **errp)
  {
@@ -204,10 +200,5 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
  
  assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);

  opts = >u.vhost_vdpa;
-/* verify net frontend */
-if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
-  (char *)name, errp)) {
-return -1;
-}
  return net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, opts->vhostdev);
  }




Re: [PATCH v2 02/15] net/vhost-user: Fix device compatibility check

2021-10-13 Thread Jason Wang



在 2021/10/8 下午9:34, Kevin Wolf 写道:

vhost-user works only with specific devices. At startup, it second
guesses what the command line option handling will do and error out if
it thinks a non-virtio device will attach to them.

This second guessing is not only ugly, it can lead to wrong error
messages ('-device floppy,netdev=foo' should complain about an unknown
property, not about the wrong kind of network device being attached) and
completely ignores hotplugging.

Drop the old checks and implement .check_peer_type() instead to fix
this. As a nice side effect, it also removes one more dependency on the
legacy QemuOpts infrastructure and even reduces the code size.

Signed-off-by: Kevin Wolf 



Acked-by: Jason Wang 



---
  net/vhost-user.c | 41 ++---
  1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 4a939124d2..b1a0247b59 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -198,6 +198,19 @@ static bool vhost_user_has_ufo(NetClientState *nc)
  return true;
  }
  
+static bool vhost_user_check_peer_type(NetClientState *nc, ObjectClass *oc,

+   Error **errp)
+{
+const char *driver = object_class_get_name(oc);
+
+if (!g_str_has_prefix(driver, "virtio-net-")) {
+error_setg(errp, "vhost-user requires frontend driver virtio-net-*");
+return false;
+}
+
+return true;
+}
+
  static NetClientInfo net_vhost_user_info = {
  .type = NET_CLIENT_DRIVER_VHOST_USER,
  .size = sizeof(NetVhostUserState),
@@ -207,6 +220,7 @@ static NetClientInfo net_vhost_user_info = {
  .has_ufo = vhost_user_has_ufo,
  .set_vnet_be = vhost_user_set_vnet_endianness,
  .set_vnet_le = vhost_user_set_vnet_endianness,
+.check_peer_type = vhost_user_check_peer_type,
  };
  
  static gboolean net_vhost_user_watch(void *do_not_use, GIOCondition cond,

@@ -397,27 +411,6 @@ static Chardev *net_vhost_claim_chardev(
  return chr;
  }
  
-static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)

-{
-const char *name = opaque;
-const char *driver, *netdev;
-
-driver = qemu_opt_get(opts, "driver");
-netdev = qemu_opt_get(opts, "netdev");
-
-if (!driver || !netdev) {
-return 0;
-}
-
-if (strcmp(netdev, name) == 0 &&
-!g_str_has_prefix(driver, "virtio-net-")) {
-error_setg(errp, "vhost-user requires frontend driver virtio-net-*");
-return -1;
-}
-
-return 0;
-}
-
  int net_init_vhost_user(const Netdev *netdev, const char *name,
  NetClientState *peer, Error **errp)
  {
@@ -433,12 +426,6 @@ int net_init_vhost_user(const Netdev *netdev, const char 
*name,
  return -1;
  }
  
-/* verify net frontend */

-if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
-  (char *)name, errp)) {
-return -1;
-}
-
  queues = vhost_user_opts->has_queues ? vhost_user_opts->queues : 1;
  if (queues < 1 || queues > MAX_QUEUE_NUM) {
  error_setg(errp,




Re: [PATCH v2 01/15] net: Introduce NetClientInfo.check_peer_type()

2021-10-13 Thread Jason Wang



在 2021/10/8 下午9:34, Kevin Wolf 写道:

Some network backends (vhost-user and vhost-vdpa) work only with
specific devices. At startup, they second guess what the command line
option handling will do and error out if they think a non-virtio device
will attach to them.

This second guessing is not only ugly, it can lead to wrong error
messages ('-device floppy,netdev=foo' should complain about an unknown
property, not about the wrong kind of network device being attached) and
completely ignores hotplugging.

Add a callback where backends can check compatibility with a device when
it actually tries to attach, even on hotplug.

Signed-off-by: Kevin Wolf 



Acked-by: Jason Wang 



---
  include/net/net.h| 2 ++
  hw/core/qdev-properties-system.c | 6 ++
  2 files changed, 8 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 5d1508081f..986288eb07 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -62,6 +62,7 @@ typedef struct SocketReadState SocketReadState;
  typedef void (SocketReadStateFinalize)(SocketReadState *rs);
  typedef void (NetAnnounce)(NetClientState *);
  typedef bool (SetSteeringEBPF)(NetClientState *, int);
+typedef bool (NetCheckPeerType)(NetClientState *, ObjectClass *, Error **);
  
  typedef struct NetClientInfo {

  NetClientDriver type;
@@ -84,6 +85,7 @@ typedef struct NetClientInfo {
  SetVnetBE *set_vnet_be;
  NetAnnounce *announce;
  SetSteeringEBPF *set_steering_ebpf;
+NetCheckPeerType *check_peer_type;
  } NetClientInfo;
  
  struct NetClientState {

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index e71f5d64d1..a91f60567a 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -431,6 +431,12 @@ static void set_netdev(Object *obj, Visitor *v, const char 
*name,
  goto out;
  }
  
+if (peers[i]->info->check_peer_type) {

+if (!peers[i]->info->check_peer_type(peers[i], obj->class, errp)) {
+goto out;
+}
+}
+
  ncs[i] = peers[i];
  ncs[i]->queue_index = i;
  }




Re: [PATCH v2 09/15] softmmu/qdev-monitor: add error handling in qdev_set_id

2021-10-13 Thread Eric Blake
On Wed, Oct 13, 2021 at 03:10:38PM +0200, Damien Hedde wrote:
> > > @@ -691,7 +703,13 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error 
> > > **errp)
> > >   }
> > >   }
> > > -qdev_set_id(dev, g_strdup(qemu_opts_id(opts)));
> > > +/*
> > > + * set dev's parent and register its id.
> > > + * If it fails it means the id is already taken.
> > > + */
> > > +if (!qdev_set_id(dev, g_strdup(qemu_opts_id(opts)), errp)) {
> > > +goto err_del_dev;
> > 
> > ...nor on this, which means the g_strdup() leaks on failure.
> > 
> 
> Since we strdup the id just before calling qdev_set_id.
> Maybe we should do the the strdup in qdev_set_id (and free things on error
> there too). It seems simplier than freeing things on the callee side just in
> case of an error.

Indeed.  If we expected qdev_set_id() to be passed something that it
can later free, we would have used 'char *'; but because we used
'const char *' for that parameter, it really does make more sense for
the callers to pass in any string and for qdev_set_id() to do the
necessary strdup()ing, as well as clean up on error.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



Re: [PATCH v2 00/15] qdev: Add JSON -device

2021-10-13 Thread Kevin Wolf
Am 13.10.2021 um 17:30 hat Michael S. Tsirkin geschrieben:
> On Fri, Oct 08, 2021 at 03:34:27PM +0200, Kevin Wolf wrote:
> > It's still a long way until we'll have QAPIfied devices, but there are
> > some improvements that we can already make now to make the future switch
> > easier.
> > 
> > One important part of this is having code paths without QemuOpts, which
> > we want to get rid of and replace with the keyval parser in the long
> > run. This series adds support for JSON syntax to -device, which bypasses
> > QemuOpts.
> > 
> > While we're not using QAPI yet, devices are based on QOM, so we already
> > do have type checks and an implied schema. JSON syntax supported now can
> > be supported by QAPI later and regarding command line compatibility,
> > actually switching to it becomes an implementation detail this way (of
> > course, it will still add valuable user-visible features like
> > introspection and documentation).
> > 
> > Apart from making things more future proof, this also immediately adds
> > a way to do non-scalar properties on the command line. nvme could have
> > used list support recently, and the lack of it in -device led to some
> > rather unnatural solution in the first version (doing the relationship
> > between a device and objects backwards) and loss of features in the
> > following. With this series, using a list as a device property should be
> > possible without any weird tricks.
> > 
> > Unfortunately, even QMP device_add goes through QemuOpts before this
> > series, which destroys any type safety QOM provides and also can't
> > support non-scalar properties. This is a bug, but it turns out that
> > libvirt actually relies on it and passes only strings for everything.
> > So this series still leaves device_add alone until libvirt is fixed.
> 
> 
> Reviewed-by: Michael S. Tsirkin 
> 
> I assume you are merging this?

Yes, I can merge it through my tree. Thanks for the review!

Kevin



Re: [PATCH v2 00/15] qdev: Add JSON -device

2021-10-13 Thread Michael S. Tsirkin
On Fri, Oct 08, 2021 at 03:34:27PM +0200, Kevin Wolf wrote:
> It's still a long way until we'll have QAPIfied devices, but there are
> some improvements that we can already make now to make the future switch
> easier.
> 
> One important part of this is having code paths without QemuOpts, which
> we want to get rid of and replace with the keyval parser in the long
> run. This series adds support for JSON syntax to -device, which bypasses
> QemuOpts.
> 
> While we're not using QAPI yet, devices are based on QOM, so we already
> do have type checks and an implied schema. JSON syntax supported now can
> be supported by QAPI later and regarding command line compatibility,
> actually switching to it becomes an implementation detail this way (of
> course, it will still add valuable user-visible features like
> introspection and documentation).
> 
> Apart from making things more future proof, this also immediately adds
> a way to do non-scalar properties on the command line. nvme could have
> used list support recently, and the lack of it in -device led to some
> rather unnatural solution in the first version (doing the relationship
> between a device and objects backwards) and loss of features in the
> following. With this series, using a list as a device property should be
> possible without any weird tricks.
> 
> Unfortunately, even QMP device_add goes through QemuOpts before this
> series, which destroys any type safety QOM provides and also can't
> support non-scalar properties. This is a bug, but it turns out that
> libvirt actually relies on it and passes only strings for everything.
> So this series still leaves device_add alone until libvirt is fixed.


Reviewed-by: Michael S. Tsirkin 

I assume you are merging this?

> v2:
> - Drop type safe QMP device_add because libvirt gets it wrong, too
> - More network patches to eliminate dependencies on the legacy QemuOpts
>   data structures which would not contain all devices any more after
>   this series. Fix some bugs there as a side effect.
> - Remove an unnecessary use of ERRP_GUARD()
> - Replaced error handling patch for qdev_set_id() with Damien's
> - Drop the deprecation patch until libvirt uses the new JSON syntax
> 
> Damien Hedde (1):
>   softmmu/qdev-monitor: add error handling in qdev_set_id
> 
> Kevin Wolf (14):
>   net: Introduce NetClientInfo.check_peer_type()
>   net/vhost-user: Fix device compatibility check
>   net/vhost-vdpa: Fix device compatibility check
>   qom: Reduce use of error_propagate()
>   iotests/245: Fix type for iothread property
>   iotests/051: Fix typo
>   qdev: Avoid using string visitor for properties
>   qdev: Make DeviceState.id independent of QemuOpts
>   qemu-option: Allow deleting opts during qemu_opts_foreach()
>   qdev: Add Error parameter to hide_device() callbacks
>   virtio-net: Store failover primary opts pointer locally
>   virtio-net: Avoid QemuOpts in failover_find_primary_device()
>   qdev: Base object creation on QDict rather than QemuOpts
>   vl: Enable JSON syntax for -device
> 
>  qapi/qdev.json  | 15 +++--
>  include/hw/qdev-core.h  | 15 +++--
>  include/hw/virtio/virtio-net.h  |  2 +
>  include/monitor/qdev.h  | 27 +++-
>  include/net/net.h   |  2 +
>  hw/arm/virt.c   |  2 +-
>  hw/core/qdev-properties-system.c|  6 ++
>  hw/core/qdev.c  | 11 +++-
>  hw/net/virtio-net.c | 85 -
>  hw/pci-bridge/pci_expander_bridge.c |  2 +-
>  hw/ppc/e500.c   |  2 +-
>  hw/vfio/pci.c   |  4 +-
>  hw/xen/xen-legacy-backend.c |  3 +-
>  net/vhost-user.c| 41 
>  net/vhost-vdpa.c| 37 ---
>  qom/object.c|  7 +-
>  qom/object_interfaces.c | 19 ++
>  softmmu/qdev-monitor.c  | 99 +++--
>  softmmu/vl.c| 63 --
>  util/qemu-option.c  |  4 +-
>  tests/qemu-iotests/051  |  2 +-
>  tests/qemu-iotests/051.pc.out   |  4 +-
>  tests/qemu-iotests/245  |  4 +-
>  23 files changed, 278 insertions(+), 178 deletions(-)
> 
> -- 
> 2.31.1



Re: [PATCH v4] qemu: tpm: Run swtpm_setup --create-config-files in session mode

2021-10-13 Thread Marc-André Lureau
Hi

On Wed, Oct 13, 2021 at 4:57 PM Stefan Berger  wrote:

> Using swtpm v0.7.0 we can run swtpm_setup to create default config files
> for swtpm_setup and swtpm-localca in session mode. Now a user can start
> a VM with an attached TPM without having to run this program on the
> command line before. This program needs to run once.
>
> This patch addresses the issue raised in
> https://bugzilla.redhat.com/show_bug.cgi?id=2010649
>
> Signed-off-by: Stefan Berger 
>
> ---
> v4:
>   - Append stderr output to virReportError if swtpm_setup fails
>
> v3:
>   - Removed logfile parameter
>
> v2:
>   - fixed return code if swtpm_setup doesn't support the option
> ---
>  src/qemu/qemu_tpm.c | 42 ++
>  src/util/virtpm.c   |  1 +
>  src/util/virtpm.h   |  1 +
>  3 files changed, 44 insertions(+)
>
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 100481503c..f797a87fdc 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -385,6 +385,45 @@ qemuTPMSetupEncryption(const unsigned char
> *secretuuid,
>  return virCommandSetSendBuffer(cmd, g_steal_pointer(),
> secret_len);
>  }
>
> +
> +/*
> + * qemuTPMCreateConfigFiles: run swtpm_setup --create-config-files
> skip-if-exist
> + */
> +static int
> +qemuTPMCreateConfigFiles(void)
> +{
> +g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> +g_autoptr(virCommand) cmd = NULL;
> +g_autofree char *errbuf = NULL;
> +int exitstatus;
> +
> +if (!swtpm_setup)
> +return -1;
> +
> +if (!virTPMSwtpmSetupCapsGet(
> +VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES))
> +return 0;
> +
> +cmd = virCommandNew(swtpm_setup);
> +if (!cmd)
> +return -1;
> +
> +virCommandAddArgList(cmd, "--create-config-files", "skip-if-exist",
> NULL);
> +virCommandClearCaps(cmd);
> +virCommandSetErrorBuffer(cmd, );
> +
> +if (virCommandRun(cmd, ) < 0 || exitstatus != 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Could not run '%s' to create config files. "
> + "exitstatus: %d;\nError: %s"),
> +  swtpm_setup, exitstatus, errbuf);
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +
>  /*
>   * qemuTPMEmulatorRunSetup
>   *
> @@ -432,6 +471,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>   "this requires privileged mode for a "
>   "TPM 1.2\n"), 0600);
>
> +if (!privileged && qemuTPMCreateConfigFiles())
> +return -1;
> +
>  cmd = virCommandNew(swtpm_setup);
>  if (!cmd)
>  return -1;
> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
> index 1a567139b4..0f50de866c 100644
> --- a/src/util/virtpm.c
> +++ b/src/util/virtpm.c
> @@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
>  VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
>VIR_TPM_SWTPM_SETUP_FEATURE_LAST,
>"cmdarg-pwdfile-fd",
> +  "cmdarg-create-config-files",
>  );
>
>  /**
> diff --git a/src/util/virtpm.h b/src/util/virtpm.h
> index d021a083b4..3bb03b3b33 100644
> --- a/src/util/virtpm.h
> +++ b/src/util/virtpm.h
> @@ -38,6 +38,7 @@ typedef enum {
>
>  typedef enum {
>  VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD,
> +VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES,
>
>  VIR_TPM_SWTPM_SETUP_FEATURE_LAST
>  } virTPMSwtpmSetupFeature;
> --
> 2.31.1
>
>
lgtm,
Reviewed-by: Marc-André Lureau 


Re: [PATCH v2 08/15] qdev: Make DeviceState.id independent of QemuOpts

2021-10-13 Thread Damien Hedde




On 10/8/21 15:34, Kevin Wolf wrote:

DeviceState.id is a pointer to a string that is stored in the QemuOpts
object DeviceState.opts and freed together with it. We want to create
devices without going through QemuOpts in the future, so make this a
separately allocated string.

Signed-off-by: Kevin Wolf 
Reviewed-by: Vladimir Sementsov-Ogievskiy 



Reviewed-by: Damien Hedde 



Re: [PATCH v2 03/15] net/vhost-vdpa: Fix device compatibility check

2021-10-13 Thread Damien Hedde




On 10/8/21 15:34, Kevin Wolf wrote:

vhost-vdpa works only with specific devices. At startup, it second
guesses what the command line option handling will do and error out if
it thinks a non-virtio device will attach to them.

This second guessing is not only ugly, it can lead to wrong error
messages ('-device floppy,netdev=foo' should complain about an unknown
property, not about the wrong kind of network device being attached) and
completely ignores hotplugging.

Drop the old checks and implement .check_peer_type() instead to fix
this. As a nice side effect, it also removes one more dependency on the
legacy QemuOpts infrastructure and even reduces the code size.

Signed-off-by: Kevin Wolf 


Reviewed-by: Damien Hedde 



Re: [PATCH v2 02/15] net/vhost-user: Fix device compatibility check

2021-10-13 Thread Damien Hedde




On 10/8/21 15:34, Kevin Wolf wrote:

vhost-user works only with specific devices. At startup, it second
guesses what the command line option handling will do and error out if
it thinks a non-virtio device will attach to them.

This second guessing is not only ugly, it can lead to wrong error
messages ('-device floppy,netdev=foo' should complain about an unknown
property, not about the wrong kind of network device being attached) and
completely ignores hotplugging.

Drop the old checks and implement .check_peer_type() instead to fix
this. As a nice side effect, it also removes one more dependency on the
legacy QemuOpts infrastructure and even reduces the code size.

Signed-off-by: Kevin Wolf 


Reviewed-by: Damien Hedde 



Re: [PATCH v2 01/15] net: Introduce NetClientInfo.check_peer_type()

2021-10-13 Thread Damien Hedde




On 10/8/21 15:34, Kevin Wolf wrote:

Some network backends (vhost-user and vhost-vdpa) work only with
specific devices. At startup, they second guess what the command line
option handling will do and error out if they think a non-virtio device
will attach to them.

This second guessing is not only ugly, it can lead to wrong error
messages ('-device floppy,netdev=foo' should complain about an unknown
property, not about the wrong kind of network device being attached) and
completely ignores hotplugging.

Add a callback where backends can check compatibility with a device when
it actually tries to attach, even on hotplug.

Signed-off-by: Kevin Wolf 


Reviewed-by: Damien Hedde 

---
  include/net/net.h| 2 ++
  hw/core/qdev-properties-system.c | 6 ++
  2 files changed, 8 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 5d1508081f..986288eb07 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -62,6 +62,7 @@ typedef struct SocketReadState SocketReadState;
  typedef void (SocketReadStateFinalize)(SocketReadState *rs);
  typedef void (NetAnnounce)(NetClientState *);
  typedef bool (SetSteeringEBPF)(NetClientState *, int);
+typedef bool (NetCheckPeerType)(NetClientState *, ObjectClass *, Error **);
  
  typedef struct NetClientInfo {

  NetClientDriver type;
@@ -84,6 +85,7 @@ typedef struct NetClientInfo {
  SetVnetBE *set_vnet_be;
  NetAnnounce *announce;
  SetSteeringEBPF *set_steering_ebpf;
+NetCheckPeerType *check_peer_type;
  } NetClientInfo;
  
  struct NetClientState {

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index e71f5d64d1..a91f60567a 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -431,6 +431,12 @@ static void set_netdev(Object *obj, Visitor *v, const char 
*name,
  goto out;
  }
  
+if (peers[i]->info->check_peer_type) {

+if (!peers[i]->info->check_peer_type(peers[i], obj->class, errp)) {
+goto out;
+}
+}
+
  ncs[i] = peers[i];
  ncs[i]->queue_index = i;
  }





Re: [PATCH v2 09/15] softmmu/qdev-monitor: add error handling in qdev_set_id

2021-10-13 Thread Damien Hedde




On 10/11/21 23:00, Eric Blake wrote:

On Fri, Oct 08, 2021 at 03:34:36PM +0200, Kevin Wolf wrote:

From: Damien Hedde 

qdev_set_id() is mostly used when the user adds a device (using
-device cli option or device_add qmp command). This commit adds
an error parameter to handle the case where the given id is
already taken.

Also document the function and add a return value in order to
be able to capture success/failure: the function now returns the
id in case of success, or NULL in case of failure.

The commit modifies the 2 calling places (qdev-monitor and
xen-legacy-backend) to add the error object parameter.

Note that the id is, right now, guaranteed to be unique because
all ids came from the "device" QemuOptsList where the id is used
as key. This addition is a preparation for a future commit which
will relax the uniqueness.

Signed-off-by: Damien Hedde 
Signed-off-by: Kevin Wolf 
---



+} else {
+error_setg(errp, "Duplicate device ID '%s'", id);
+return NULL;


id is not freed on this error path...



  
  DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)

@@ -691,7 +703,13 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
  }
  }
  
-qdev_set_id(dev, g_strdup(qemu_opts_id(opts)));

+/*
+ * set dev's parent and register its id.
+ * If it fails it means the id is already taken.
+ */
+if (!qdev_set_id(dev, g_strdup(qemu_opts_id(opts)), errp)) {
+goto err_del_dev;


...nor on this, which means the g_strdup() leaks on failure.



Since we strdup the id just before calling qdev_set_id.
Maybe we should do the the strdup in qdev_set_id (and free things on 
error there too). It seems simplier than freeing things on the callee 
side just in case of an error.



Damien






[PATCH v4] qemu: tpm: Run swtpm_setup --create-config-files in session mode

2021-10-13 Thread Stefan Berger
Using swtpm v0.7.0 we can run swtpm_setup to create default config files
for swtpm_setup and swtpm-localca in session mode. Now a user can start
a VM with an attached TPM without having to run this program on the
command line before. This program needs to run once.

This patch addresses the issue raised in
https://bugzilla.redhat.com/show_bug.cgi?id=2010649

Signed-off-by: Stefan Berger 

---
v4:
  - Append stderr output to virReportError if swtpm_setup fails

v3:
  - Removed logfile parameter

v2:
  - fixed return code if swtpm_setup doesn't support the option
---
 src/qemu/qemu_tpm.c | 42 ++
 src/util/virtpm.c   |  1 +
 src/util/virtpm.h   |  1 +
 3 files changed, 44 insertions(+)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 100481503c..f797a87fdc 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -385,6 +385,45 @@ qemuTPMSetupEncryption(const unsigned char *secretuuid,
 return virCommandSetSendBuffer(cmd, g_steal_pointer(), secret_len);
 }
 
+
+/*
+ * qemuTPMCreateConfigFiles: run swtpm_setup --create-config-files 
skip-if-exist
+ */
+static int
+qemuTPMCreateConfigFiles(void)
+{
+g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
+g_autoptr(virCommand) cmd = NULL;
+g_autofree char *errbuf = NULL;
+int exitstatus;
+
+if (!swtpm_setup)
+return -1;
+
+if (!virTPMSwtpmSetupCapsGet(
+VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES))
+return 0;
+
+cmd = virCommandNew(swtpm_setup);
+if (!cmd)
+return -1;
+
+virCommandAddArgList(cmd, "--create-config-files", "skip-if-exist", NULL);
+virCommandClearCaps(cmd);
+virCommandSetErrorBuffer(cmd, );
+
+if (virCommandRun(cmd, ) < 0 || exitstatus != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Could not run '%s' to create config files. "
+ "exitstatus: %d;\nError: %s"),
+  swtpm_setup, exitstatus, errbuf);
+return -1;
+}
+
+return 0;
+}
+
+
 /*
  * qemuTPMEmulatorRunSetup
  *
@@ -432,6 +471,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
  "this requires privileged mode for a "
  "TPM 1.2\n"), 0600);
 
+if (!privileged && qemuTPMCreateConfigFiles())
+return -1;
+
 cmd = virCommandNew(swtpm_setup);
 if (!cmd)
 return -1;
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 1a567139b4..0f50de866c 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
 VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
   VIR_TPM_SWTPM_SETUP_FEATURE_LAST,
   "cmdarg-pwdfile-fd",
+  "cmdarg-create-config-files",
 );
 
 /**
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index d021a083b4..3bb03b3b33 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -38,6 +38,7 @@ typedef enum {
 
 typedef enum {
 VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD,
+VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES,
 
 VIR_TPM_SWTPM_SETUP_FEATURE_LAST
 } virTPMSwtpmSetupFeature;
-- 
2.31.1



Re: VM paused during multipath iSCSI reservation

2021-10-13 Thread Vojtech Juranek
I'm sorry, I send it here by mistake. Wanted to send it to libvirt-user list.
Sorry for that.
Vojta

On Wednesday, 13 October 2021 14:21:02 CEST you wrote:
> Hi,
> I'm trying to find the root cause for BZ #1898049 [1]. When setting up
> Windows HA cluster on Windows Server VMs run on top of oVirt, Windows
> cluster validator runs couple of tests and fails during test "Validate
> SCSI-3 Persistent Reservation" and one of the VMs of the cluster is paused
> with IO error. Disk definition is as follows:
> 
> 
> 
>io='native'/>  index='1'> 
> 
>path='/var/lib/libvirt/qemu/domain-1-Windows-2016-2/pr-helper0.sock'
> mode='client'/> 
>   
>   
>   
>   
>   
>   
> 
> 
> and libvirt error I get is bellow [2].
> 
> When I try to create reservation from Windows VM manually, I get following
> error (but not sure I do it whole process correctly):
> 
> .\sg_persist --out --register --param-sark=123abc e:
> QEMU   QEMU HARDDISK   2.5+
> Peripheral device type: disk
> PR out (Register): command not supported
> sg_persist failed: Illegal request, Invalid opcode
> 
> 
> Do you have any ideas what could be wrong or how to determine
> the root cause of this this issue?
> 
> Thanks in advance.
> Vojta
> 
> 
> [1] https://bugzilla.redhat.com/1898049
> [2] libvirt debug log:
> 
> 2021-10-12 11:43:25.148+: 2006427: debug : qemuMonitorEmitIOError:1243 :
> mon=0x7fb02006a020 2021-10-12 11:43:25.148+: 2006427: info :
> virObjectRef:402 : OBJECT_REF: obj=0x7fb02006a020 2021-10-12
> 11:43:25.148+: 2006427: info : virObjectRef:402 : OBJECT_REF:
> obj=0x7fafd0130020 2021-10-12 11:43:25.148+: 2000208: info :
> virObjectRef:402 : OBJECT_REF: obj=0x7fafd010d340 2021-10-12
> 11:43:25.148+: 2006427: info : virObjectNew:258 : OBJECT_NEW:
> obj=0x7fb020082590 classname=virDomainEventIOError 2021-10-12
> 11:43:25.148+: 2000208: info : vir_object_finalize:321 :
> OBJECT_DISPOSE: obj=0x7fb020082500 2021-10-12 11:43:25.148+: 2006427:
> info : virObjectNew:258 : OBJECT_NEW: obj=0x7fb020082620
> classname=virDomainEventIOError 2021-10-12 11:43:25.148+: 2000208: info
> : virObjectUnref:380 : OBJECT_UNREF: obj=0x7fb020082500 2021-10-12
> 11:43:25.148+: 2006427: debug : qemuProcessHandleIOError:907 :
> Transitioned guest Windows-2016-2 to paused state due to IO error
> 2021-10-12 11:43:25.148+: 2000208: info : virObjectUnref:380 :
> OBJECT_UNREF: obj=0x7fafd010d340 2021-10-12 11:43:25.148+: 2006427:
> info : virObjectNew:258 : OBJECT_NEW: obj=0x7fafbc1fb8c0
> classname=virDomainEventLifecycle 2021-10-12 11:43:25.148+: 2006427:
> debug : virDomainLockProcessPause:204 : plugin=0x7fafd01272a0
> dom=0x7fb01400f5e0 state=0x7fb02401d768 2021-10-12 11:43:25.148+:
> 2006427: debug : virDomainLockManagerNew:134 : plugin=0x7fafd01272a0
> dom=0x7fb01400f5e0 withResources=1 2021-10-12 11:43:25.148+: 2006427:
> debug : virLockManagerPluginGetDriver:276 : plugin=0x7fafd01272a0
> 2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerNew:300 :
> driver=0x7fafd444a000 type=0 nparams=5 params=0x7fafd77de640 flags=0x0
> 2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerLogParams:97 :
>   key=uuid type=uuid value=70eee88c-ba2c-4c6c-bd51-c2b663db27f8 2021-10-12
> 11:43:25.148+: 2006427: debug : virLockManagerLogParams:90 :   key=name
> type=string value=Windows-2016-2 2021-10-12 11:43:25.148+: 2006427:
> debug : virLockManagerLogParams:78 :   key=id type=uint value=1 2021-10-12
> 11:43:25.148+: 2006427: debug : virLockManagerLogParams:78 :   key=pid
> type=uint value=2006418 2021-10-12 11:43:25.148+: 2006427: debug :
> virLockManagerLogParams:93 :   key=uri type=cstring value=(null) 2021-10-12
> 11:43:25.148+: 2006427: debug : virDomainLockManagerNew:146 : Adding
> leases 2021-10-12 11:43:25.148+: 2006427: debug :
> virDomainLockManagerNew:151 : Adding disks 2021-10-12 11:43:25.149+:
> 2006427: debug : virDomainLockManagerAddImage:90 : Add disk
> /rhev/data-center/mnt/blockSD/7c4f09b6-9e87-436f-bda9-22d1f0b50955/images/f
> 5d6e074-dfe9-462d-8cfd-3e14b0eb5aea/766e36b2-84a6-43e7-a48b-a5f47e669860
> 2021-10-12 11:43:25.149+: 2006427: debug :
> virLockManagerAddResource:326 : lock=0x7fafbc19e250 type=0
> name=/rhev/data-center/mnt/blockSD/7c4f09b6-9e87-436f-bda9-22d1f0b50955/ima
> ges/f5d6e074-dfe9-462d-8cfd-3e14b0eb5aea/766e36b2-84a6-43e7-a48b-a5f47e66986
> 0 nparams=0 params=(nil) flags=0x0 2021-10-12 11:43:25.149+: 2006427:
> debug : virDomainLockManagerAddImage:90 : Add disk
> /dev/mapper/3600a09803830447a4f244c4657616f6f 2021-10-12 11:43:25.149+:
> 2006427: debug : virLockManagerAddResource:326 : lock=0x7fafbc19e250 type=0
> name=/dev/mapper/3600a09803830447a4f244c4657616f6f nparams=0 params=(nil)
> flags=0x2 2021-10-12 11:43:25.149+: 2006427: debug :
> virLockManagerRelease:359 : lock=0x7fafbc19e250 state=0x7fb02401d768
> flags=0x0 2021-10-12 11:43:25.149+: 2006427: 

Re: [PATCH] apparmor: ceph config file names

2021-10-13 Thread Jamie Strandboge
On Thu, 07 Oct 2021, christian.ehrha...@canonical.com wrote:

> From: Christian Ehrhardt 
> 
> If running multiple [1] clusters (uncommon) the ceph config file will be
> derived from the cluster name. Therefore the rule to allow to read ceph
> config files need to be opened up slightly to allow for that condition.
> 
> [1]: 
> https://docs.ceph.com/en/mimic/rados/configuration/common/#running-multiple-clusters
> 
> Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1588576
> 
> Signed-off-by: Christian Ehrhardt 
> ---
>  src/security/apparmor/libvirt-qemu | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/security/apparmor/libvirt-qemu 
> b/src/security/apparmor/libvirt-qemu
> index 4156428163..8cd76d48ec 100644
> --- a/src/security/apparmor/libvirt-qemu
> +++ b/src/security/apparmor/libvirt-qemu
> @@ -199,7 +199,7 @@
>/sys/class/ r,
>  
># for rbd
> -  /etc/ceph/ceph.conf r,
> +  /etc/ceph/*.conf r,
>  
># Various functions will need to enumerate /tmp (e.g. ceph), allow the base
># dir and a few known functions like samba support.

LGTM, thanks!

-- 
Email: ja...@strandboge.com
IRC:   jdstrand


signature.asc
Description: PGP signature


VM paused during multipath iSCSI reservation

2021-10-13 Thread Vojtech Juranek
Hi,
I'm trying to find the root cause for BZ #1898049 [1]. When setting up Windows 
HA 
cluster on Windows Server VMs run on top of oVirt, Windows cluster validator 
runs couple of tests 
and fails during test "Validate SCSI-3 Persistent Reservation" and one of the 
VMs of 
the cluster is paused with IO error. Disk definition is as follows:



  
  


  

  
  
  
  
  
  


and libvirt error I get is bellow [2].

When I try to create reservation from Windows VM manually, I get following error
(but not sure I do it whole process correctly):

.\sg_persist --out --register --param-sark=123abc e:
QEMU   QEMU HARDDISK   2.5+
Peripheral device type: disk
PR out (Register): command not supported
sg_persist failed: Illegal request, Invalid opcode


Do you have any ideas what could be wrong or how to determine
the root cause of this this issue?

Thanks in advance.
Vojta


[1] https://bugzilla.redhat.com/1898049
[2] libvirt debug log:

2021-10-12 11:43:25.148+: 2006427: debug : qemuMonitorEmitIOError:1243 : 
mon=0x7fb02006a020
2021-10-12 11:43:25.148+: 2006427: info : virObjectRef:402 : OBJECT_REF: 
obj=0x7fb02006a020
2021-10-12 11:43:25.148+: 2006427: info : virObjectRef:402 : OBJECT_REF: 
obj=0x7fafd0130020
2021-10-12 11:43:25.148+: 2000208: info : virObjectRef:402 : OBJECT_REF: 
obj=0x7fafd010d340
2021-10-12 11:43:25.148+: 2006427: info : virObjectNew:258 : OBJECT_NEW: 
obj=0x7fb020082590 classname=virDomainEventIOError
2021-10-12 11:43:25.148+: 2000208: info : vir_object_finalize:321 : 
OBJECT_DISPOSE: obj=0x7fb020082500
2021-10-12 11:43:25.148+: 2006427: info : virObjectNew:258 : OBJECT_NEW: 
obj=0x7fb020082620 classname=virDomainEventIOError
2021-10-12 11:43:25.148+: 2000208: info : virObjectUnref:380 : 
OBJECT_UNREF: obj=0x7fb020082500
2021-10-12 11:43:25.148+: 2006427: debug : qemuProcessHandleIOError:907 : 
Transitioned guest Windows-2016-2 to paused state due to IO error
2021-10-12 11:43:25.148+: 2000208: info : virObjectUnref:380 : 
OBJECT_UNREF: obj=0x7fafd010d340
2021-10-12 11:43:25.148+: 2006427: info : virObjectNew:258 : OBJECT_NEW: 
obj=0x7fafbc1fb8c0 classname=virDomainEventLifecycle
2021-10-12 11:43:25.148+: 2006427: debug : virDomainLockProcessPause:204 : 
plugin=0x7fafd01272a0 dom=0x7fb01400f5e0 state=0x7fb02401d768
2021-10-12 11:43:25.148+: 2006427: debug : virDomainLockManagerNew:134 : 
plugin=0x7fafd01272a0 dom=0x7fb01400f5e0 withResources=1
2021-10-12 11:43:25.148+: 2006427: debug : 
virLockManagerPluginGetDriver:276 : plugin=0x7fafd01272a0
2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerNew:300 : 
driver=0x7fafd444a000 type=0 nparams=5 params=0x7fafd77de640 flags=0x0
2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerLogParams:97 :   
key=uuid type=uuid value=70eee88c-ba2c-4c6c-bd51-c2b663db27f8
2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerLogParams:90 :   
key=name type=string value=Windows-2016-2
2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerLogParams:78 :   
key=id type=uint value=1
2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerLogParams:78 :   
key=pid type=uint value=2006418
2021-10-12 11:43:25.148+: 2006427: debug : virLockManagerLogParams:93 :   
key=uri type=cstring value=(null)
2021-10-12 11:43:25.148+: 2006427: debug : virDomainLockManagerNew:146 : 
Adding leases
2021-10-12 11:43:25.148+: 2006427: debug : virDomainLockManagerNew:151 : 
Adding disks
2021-10-12 11:43:25.149+: 2006427: debug : virDomainLockManagerAddImage:90 
: Add disk 
/rhev/data-center/mnt/blockSD/7c4f09b6-9e87-436f-bda9-22d1f0b50955/images/f5d6e074-dfe9-462d-8cfd-3e14b0eb5aea/766e36b2-84a6-43e7-a48b-a5f47e669860
2021-10-12 11:43:25.149+: 2006427: debug : virLockManagerAddResource:326 : 
lock=0x7fafbc19e250 type=0 
name=/rhev/data-center/mnt/blockSD/7c4f09b6-9e87-436f-bda9-22d1f0b50955/images/f5d6e074-dfe9-462d-8cfd-3e14b0eb5aea/766e36b2-84a6-43e7-a48b-a5f47e669860
 nparams=0 params=(nil) flags=0x0
2021-10-12 11:43:25.149+: 2006427: debug : virDomainLockManagerAddImage:90 
: Add disk /dev/mapper/3600a09803830447a4f244c4657616f6f
2021-10-12 11:43:25.149+: 2006427: debug : virLockManagerAddResource:326 : 
lock=0x7fafbc19e250 type=0 name=/dev/mapper/3600a09803830447a4f244c4657616f6f 
nparams=0 params=(nil) flags=0x2
2021-10-12 11:43:25.149+: 2006427: debug : virLockManagerRelease:359 : 
lock=0x7fafbc19e250 state=0x7fb02401d768 flags=0x0
2021-10-12 11:43:25.149+: 2006427: debug : virLockManagerFree:381 : 
lock=0x7fafbc19e250
2021-10-12 11:43:25.149+: 2006427: debug : qemuProcessHandleIOError:920 : 
Preserving lock state ''
2021-10-12 11:43:25.150+: 2006427: info : virObjectUnref:380 : 
OBJECT_UNREF: obj=0x7fafd0130020
2021-10-12 11:43:25.150+: 2006427: info : virObjectUnref:380 : 
OBJECT_UNREF: obj=0x7fb02006a020
2021-10-12 11:43:25.150+: 2000208: info 

Re: [libvirt PATCH 6/7] nodedev: Handle inactive mdevs with the same UUID

2021-10-13 Thread Boris Fiuczynski

On 7/26/21 4:47 PM, Michal Prívozník wrote:

On 7/23/21 6:40 PM, Jonathon Jongsma wrote:

Unfortunately, mdevctl supports defining more than one mdev with the
same UUID as long as they have different parent devices. (Only one of
these devices can be active at any given time).

This means that we can't use the UUID alone as a way to uniquely
identify mdev node devices. Append the parent address to ensure
uniqueness. For example:

 Before: mdev_88a6b868_46bd_4015_8e5b_26107f82da38
 After:  mdev_88a6b868_46bd_4015_8e5b_26107f82da38__00_02_0

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1979440

Signed-off-by: Jonathon Jongsma 
---
  src/node_device/node_device_driver.c   | 3 ++-
  src/node_device/node_device_udev.c | 2 +-
  tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml | 8 
  3 files changed, 7 insertions(+), 6 deletions(-)


The patch looks good, but for some reason it leaves API breakage
aftertaste. But I guess we don't document anywhere what MDEV 
looks like, do we? IOW - we are free to change it if needed.

Michal



Michal, Jonathon,
now that the name change broke the undocumented {bus}_{device} format 
pattern I am coping with the aftermath.
Besides the fact that before- and after-format pattern support cannot 
easily be introspected until actually dealing with an mdev and than try 
to make sense from its name I am confronted with two general questions:


1. How stable are nodedev object names?

2. Do nodedev object names contain bus and device address information 
one can rely on or are they (since the format is undocumented) just 
arbitrary characters?



--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




Re: [RFC PATCH] qemu: Do not report eof when processing monitor IO

2021-10-13 Thread Michal Prívozník
On 10/8/21 11:37 PM, Jim Fehlig wrote:
> There have been countless reports from users concerned about the following
> error reported by libvirtd when qemu domains are shutdown
> 
> internal error: End of file from qemu monitor
> 
> While the error is harmless, users often mistaken it for real problem with
> their deployments. EOF from the monitor can't be entirely ignored since
> other threads may be using the monitor and must be able to detect the EOF
> condition.
> 
> One potential fix is to delay reporting EOF until the monitor is used
> after EOF is detected. This patch adds a 'goteof' member to the
> qemuMonitor structure, which is set when EOF is detected on the monitor
> socket. If another thread later tries to send data on the monitor, the
> EOF error is reported.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> An RFC patch to squelch qemu monitor EOF error messages on VM shutdown.
> Previous discussions and information on testing of the patch can be
> found in this thread
> 
> https://listman.redhat.com/archives/libvir-list/2021-September/msg00949.html
> 
>  src/qemu/qemu_monitor.c | 29 -
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 5fc23f13d3..751ec8ba6c 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -113,6 +113,7 @@ struct _qemuMonitor {
>  
>  /* true if qemu no longer wants 'props' sub-object of object-add */
>  bool objectAddNoWrap;
> +bool goteof;
>  };
>  
>  /**
> @@ -526,10 +527,10 @@ qemuMonitorIO(GSocket *socket G_GNUC_UNUSED,
>  {
>  qemuMonitor *mon = opaque;
>  bool error = false;
> -bool eof = false;
>  bool hangup = false;
>  
>  virObjectRef(mon);
> +mon->goteof = false;
>  

At this point, the monitor object is unlocked (see the line below). So
setting this flag outside is potentially dangerous. But, I don't think
we need to set ->goteof here at all, do we? I mean, the moment we see
EOF the monitor object will be disposed and not ever used again.

>  /* lock access to the monitor and protect fd */
>  virObjectLock(mon);

Otherwise looking good and also passes my testing.

Michal



Re: [PATCH 0/2] qemu_migration: set priv->migMaxBandwidth during migration

2021-10-13 Thread Michal Prívozník
On 10/8/21 10:19 AM, Kristina Hanicova wrote:
> 
> Kristina Hanicova (2):
>   qemu_migration: set bandwidth in priv during migration
>   qemu_migration: drop unnecessary 'migrate_speed' variable
> 
>  src/qemu/qemu_migration.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 

Pushed now.

Michal



Re: [PATCH v2] virsh: Add QMP command wrapping for 'qemu-monitor-command'

2021-10-13 Thread Peter Krempa
On Fri, Sep 17, 2021 at 15:34:56 +0200, Peter Krempa wrote:
> Issuing simple QMP commands is pain as they need to be wrapped by the
> JSON wrapper:
> 
>  { "execute": "COMMAND" }
> 
> and optionally also:
> 
>  { "execute": "COMMAND", "arguments":...}
> 
> For simple commands without arguments we can add syntax sugar to virsh
> which allows simple usage of QMP and additionally prepares also for
> passing through of the 'arguments' section:
> 
>  virsh qemu-monitor-command $VM query-status
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-status"}'
> 
> and
> 
>  virsh qemu-monitor-command $VM query-named-block-nodes '{"flat":true}'
>  or
>  virsh qemu-monitor-command $VM query-named-block-nodes '"flat":true'
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-named-block-nodes", 
> "arguments":{"flat":true}}'
> 
> Signed-off-by: Peter Krempa 
> ---
> 
> v2:
>   - dropped the '--qmpwrap' option and do wrapping if we don't get a
>   JSON object instead. Similarly for arguments.
> 
>  docs/manpages/virsh.rst | 16 ++-
>  tools/virsh-domain.c| 98 -
>  2 files changed, 103 insertions(+), 11 deletions(-)

Ping? :)