Re: [libvirt PATCH v2 3/3] qemu: add vdpa support

2020-09-03 Thread Jonathon Jongsma
On Thu, 2020-09-03 at 00:55 -0400, Laine Stump wrote:
> On 9/2/20 3:25 PM, Jonathon Jongsma wrote:
> > Enable  for qemu domains. This provides
> > basic
> > support and does not support hotplug or migration.
> 
> Is there something specifically preventing hotplug, or you just
> haven't 
> implemented it yet?

I don't think there's anything preventing it. I avoided it for this
first patch in order to keep things simpler, and because I am currently
unable to test with actual hardware...

> 
> (How does that work with FD passing, anyway? I haven't really looked
> at 
> the details of FD passing...)

I believe it requires first calling the qmp add-fd command to register
an fd with a /dev/fdset/N "filename". Then I can use the /dev/fdset/N
for the netdev_add qmp command. 

It's a little less straightforward than doing it launch time. At launch
time you have an array of all of the fds that are passed to qemu via
virCommandPassFD() and you can register the fdset id 'N' based on the
array index. But at runtime, you don't know what fdsets have already
been registered with the current qemu process. But it should be doable.
I had actually started working on that before sending out this series.
I'll try to finish it up.


> 
> If hotplug is possible, then I really think that needs to be
> implemented 
> in the initial patch set.
> 
> 
> Same question for migration - is it not possible with these devices,
> or 
> we're just not certain or are missing some pieces so we're disabling
> it 
> out of caution? For migration it's even more important to implement
> it 
> with the initial patches if it works at all. Otherwise, once we do 
> implement it, we'll have to have a way of detecting whether or not
> the 
> destination of a migration supports migrating vdpa devices. (I can
> see 
> where it simply may not work, since I guess the hardware's memory is 
> used for packet buffers, right?)

Unfortunately, I'm not sure about this. I'll have to ask around.

Jonathon



Re: [libvirt PATCH v2 3/3] qemu: add vdpa support

2020-09-02 Thread Laine Stump

On 9/2/20 3:25 PM, Jonathon Jongsma wrote:

Enable  for qemu domains. This provides basic
support and does not support hotplug or migration.



Is there something specifically preventing hotplug, or you just haven't 
implemented it yet?



(How does that work with FD passing, anyway? I haven't really looked at 
the details of FD passing...)



If hotplug is possible, then I really think that needs to be implemented 
in the initial patch set.



Same question for migration - is it not possible with these devices, or 
we're just not certain or are missing some pieces so we're disabling it 
out of caution? For migration it's even more important to implement it 
with the initial patches if it works at all. Otherwise, once we do 
implement it, we'll have to have a way of detecting whether or not the 
destination of a migration supports migrating vdpa devices. (I can see 
where it simply may not work, since I guess the hardware's memory is 
used for packet buffers, right?)





Signed-off-by: Jonathon Jongsma 
---
  src/qemu/qemu_command.c   | 28 --
  src/qemu/qemu_command.h   |  3 +-
  src/qemu/qemu_domain.c|  6 +++
  src/qemu/qemu_hotplug.c   | 12 +++---
  src/qemu/qemu_interface.c | 23 
  src/qemu/qemu_interface.h |  2 +
  src/qemu/qemu_migration.c | 10 -
  .../net-vdpa.x86_64-latest.args   | 37 +++
  tests/qemuxml2argvmock.c  | 11 +-
  tests/qemuxml2argvtest.c  |  1 +
  10 files changed, 122 insertions(+), 11 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7b7176eb72..b9830292ee 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3553,7 +3553,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
  size_t tapfdSize,
  char **vhostfd,
  size_t vhostfdSize,
-const char *slirpfd)
+const char *slirpfd,
+const char *vdpadev)
  {
  bool is_tap = false;
  virDomainNetType netType = virDomainNetGetActualType(net);
@@ -3692,6 +3693,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
  break;
  
  case VIR_DOMAIN_NET_TYPE_VDPA:

+/* Caller will pass the fd to qemu with add-fd */
+if (virJSONValueObjectCreate(, "s:type", "vhost-vdpa", NULL) 
< 0 ||
+virJSONValueObjectAppendString(netprops, "vhostdev", vdpadev) < 0)
+return NULL;
+break;
+
  case VIR_DOMAIN_NET_TYPE_HOSTDEV:
  /* Should have been handled earlier via PCI/USB hotplug code. */
  case VIR_DOMAIN_NET_TYPE_LAST:
@@ -8017,6 +8024,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
  char **tapfdName = NULL;
  char **vhostfdName = NULL;
  g_autofree char *slirpfdName = NULL;
+g_autofree char *vdpafdName = NULL;
+int vdpafd = -1;
  virDomainNetType actualType = virDomainNetGetActualType(net);
  const virNetDevBandwidth *actualBandwidth;
  bool requireNicdev = false;
@@ -8102,13 +8111,17 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
  
  break;
  
+case VIR_DOMAIN_NET_TYPE_VDPA:

+if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
+goto cleanup;
+break;
+
  case VIR_DOMAIN_NET_TYPE_USER:
  case VIR_DOMAIN_NET_TYPE_SERVER:
  case VIR_DOMAIN_NET_TYPE_CLIENT:
  case VIR_DOMAIN_NET_TYPE_MCAST:
  case VIR_DOMAIN_NET_TYPE_INTERNAL:
  case VIR_DOMAIN_NET_TYPE_UDP:
-case VIR_DOMAIN_NET_TYPE_VDPA:
  case VIR_DOMAIN_NET_TYPE_LAST:
  /* nada */
  break;
@@ -8225,13 +8238,22 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
  vhostfd[i] = -1;
  }
  
+if (vdpafd > 0) {

+virCommandPassFD(cmd, vdpafd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+g_autofree char *fdset = qemuVirCommandGetFDSet(cmd, vdpafd);
+if (!fdset)
+goto cleanup;
+virCommandAddArgList(cmd, "-add-fd", fdset, NULL);
+vdpafdName = qemuVirCommandGetDevSet(cmd, vdpafd);
+}
+
  if (chardev)
  virCommandAddArgList(cmd, "-chardev", chardev, NULL);
  
  if (!(hostnetprops = qemuBuildHostNetStr(net,

   tapfdName, tapfdSize,
   vhostfdName, vhostfdSize,
- slirpfdName)))
+ slirpfdName, vdpafdName)))
  goto cleanup;
  
  if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops,

diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 89d99b111f..8db51f93b1 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -99,7