[PATCH] qrap: Support JSON syntax for -device

[PATCH] qrap: Support JSON syntax for -device

[Andrea Bolognani]

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@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;
 	}
 
-- 
@@ -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

Re: [PATCH] qrap: Support JSON syntax for -device

[Stefano Brivio]

Hi Andrea,

On Thu, 20 Oct 2022 11:04:19 +0200
Andrea Bolognani <abologna@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@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

Re: [PATCH] qrap: Support JSON syntax for -device

[Andrea Bolognani]

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 users

In 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

Re: [PATCH] qrap: Support JSON syntax for -device

[Stefano Brivio]

[Cc: Laine]

On Mon, 24 Oct 2022 03:08:47 -0700
Andrea Bolognani <abologna@redhat.com> wrote:

> 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 users  
> 
> In addition to QEMU, we also need libvirt to support passt natively.
> 
> I believe there's ongoing work for that as well,

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).

> 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