Starting with version 8.1.0, libvirt uses JSON syntax when generating the arguments to -device, so they will now look like {"driver":"virtio-scsi-pci","bus":"pci.3","addr":"0x0"} instead of virtio-scsi-pci,bus=pci.3,addr=0x0 qrap needs to parse these arguments and extract the bus number in order to figure out what address to use for the virtio-net device it adds, and the libvirt change described above has broken this parsing logic. Tweak the code so that both styles are accepted and handled correctly. Note that, when JSON is in use, qrap needs to generate its own command line options in that format as well or things will not work as expected. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- qrap.c | 51 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/qrap.c b/qrap.c index a9a0fc1..3c6f5b8 100644 --- a/qrap.c +++ b/qrap.c @@ -69,6 +69,8 @@ static const struct drop_arg { * @name: Device ("-device") name to insert * @template: Prefix for device specification (first part of address) * @template_post: Suffix for device specification (last part of address) + * @template_json: Device prefix for when JSON is used + * @template_json_post: Device suffix for when JSON is used * @first: First usable PCI address * @last: Last usable PCI address */ @@ -77,15 +79,29 @@ static const struct pci_dev { char *name; char *template; char *template_post; + char *template_json; + char *template_json_post; int first; int last; } pci_devs[] = { - { "pc-q35", "virtio-net-pci", - "bus=pci.", ",addr=0x0", 3, /* 2: hotplug bus */ 16 }, - { "pc-", "virtio-net-pci", - "bus=pci.0,addr=0x", "", 2, /* 1: ISA bridge */ 16 }, - { "s390-ccw", "virtio-net-ccw", - "devno=fe.0.", "", 1, 16 }, + { + "pc-q35", "virtio-net-pci", + "bus=pci.", ",addr=0x0", + "\"bus\":\"pci.", ",\"addr\":\"0x0\"", + 3, /* 2: hotplug bus */ 16 + }, + { + "pc-", "virtio-net-pci", + "bus=pci.0,addr=0x", "", + "\"bus\":\"pci.0\",\"addr=0x", "", + 2, /* 1: ISA bridge */ 16 + }, + { + "s390-ccw", "virtio-net-ccw", + "devno=fe.0.", "", + "\"devno\":\"fe.0.", "", + 1, 16 + }, { 0 }, }; @@ -115,7 +131,7 @@ void usage(const char *name) */ int main(int argc, char **argv) { - int i, s, qemu_argc = 0, addr_map = 0, has_dev = 0, retry_on_reset, rc; + int i, s, qemu_argc = 0, addr_map = 0, has_dev = 0, has_json = 0, retry_on_reset, rc; struct timeval tv = { .tv_sec = 0, .tv_usec = (long)(500 * 1000) }; char *qemu_argv[ARG_MAX], dev_str[ARG_MAX]; struct sockaddr_un addr = { @@ -227,14 +243,22 @@ int main(int argc, char **argv) } if (!strcmp(argv[i], "-device") && i + 1 < argc) { + char *template = NULL; char *p; has_dev = 1; if ((p = strstr(argv[i + 1], dev->template))) { + template = dev->template; + } else if ((p = strstr(argv[i + 1], dev->template_json))) { + template = dev->template_json; + has_json = 1; + } + + if (template) { long n; - n = strtol(p + strlen(dev->template), NULL, 16); + n = strtol(p + strlen(template), NULL, 16); if (!errno) addr_map |= (1 << n); } @@ -254,8 +278,15 @@ int main(int argc, char **argv) if (has_dev) { qemu_argv[qemu_argc++] = "-device"; - snprintf(dev_str, ARG_MAX, "%s,%s%x%s,netdev=hostnet0,x-txburst=4096", - dev->name, dev->template, i, dev->template_post); + if (!has_json) { + snprintf(dev_str, ARG_MAX, + "%s,%s%x%s,netdev=hostnet0,x-txburst=4096", + dev->name, dev->template, i, dev->template_post); + } else { + snprintf(dev_str, ARG_MAX, + "{\"driver\":\"%s\",%s%x\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}", + dev->name, dev->template_json, i, dev->template_json_post); + } qemu_argv[qemu_argc++] = dev_str; } -- 2.37.3
Hi Andrea, On Thu, 20 Oct 2022 11:04:19 +0200 Andrea Bolognani <abologna(a)redhat.com> wrote:Starting with version 8.1.0, libvirt uses JSON syntax when generating the arguments to -device, so they will now look like {"driver":"virtio-scsi-pci","bus":"pci.3","addr":"0x0"} instead of virtio-scsi-pci,bus=pci.3,addr=0x0 qrap needs to parse these arguments and extract the bus number in order to figure out what address to use for the virtio-net device it adds, and the libvirt change described above has broken this parsing logic. Tweak the code so that both styles are accepted and handled correctly. Note that, when JSON is in use, qrap needs to generate its own command line options in that format as well or things will not work as expected. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>Thanks for the patch, and welcome to the git log! For context, qrap will finally become useless once this patchset by Laurent: https://patchew.org/QEMU/20221021090922.170074-1-lvivier@redhat.com/ lands in qemu -- that should happen soon. However, we'll need to keep it around for a little longer, until an updated version of qemu reaches distributions, and, after that, still give a bit of time to users, so this is absolutely useful and appreciated. I just applied it. -- Stefano
On Sat, Oct 22, 2022 at 10:19:43AM +0200, Stefano Brivio wrote:For context, qrap will finally become useless once this patchset by Laurent: https://patchew.org/QEMU/20221021090922.170074-1-lvivier@redhat.com/ lands in qemu -- that should happen soon. However, we'll need to keep it around for a little longer, until an updated version of qemu reaches distributions, and, after that, still give a bit of time to usersIn addition to QEMU, we also need libvirt to support passt natively. I believe there's ongoing work for that as well, but until that lands (and makes its way to distros) projects like KubeVirt are going to be relying on qrap.I just applied it.Thanks a lot! -- Andrea Bolognani / Red Hat / Virtualization
[Cc: Laine] On Mon, 24 Oct 2022 03:08:47 -0700 Andrea Bolognani <abologna(a)redhat.com> wrote:On Sat, Oct 22, 2022 at 10:19:43AM +0200, Stefano Brivio wrote:Yes, I'm currently working on this topic with Laine (the patch in contrib/libvirt is using the qemu interface I implemented in contrib/qemu, which is not what we'll have in qemu eventually).For context, qrap will finally become useless once this patchset by Laurent: https://patchew.org/QEMU/20221021090922.170074-1-lvivier@redhat.com/ lands in qemu -- that should happen soon. However, we'll need to keep it around for a little longer, until an updated version of qemu reaches distributions, and, after that, still give a bit of time to usersIn addition to QEMU, we also need libvirt to support passt natively. I believe there's ongoing work for that as well,but until that lands (and makes its way to distros) projects like KubeVirt are going to be relying on qrap.Right, it probably makes no sense to force KubeVirt to adopt this in two steps (passing qemu specific options first, and then switch to the new configuration model in libvirt), so we'll need to wait for that as well. -- Stefano