Re: [libvirt] [PATCH v3 2/4] qemu: add dtb capability
Hi Eric, Thanks for the comments. Could you please help review the next version? Best Regards, Olivia > -Original Message- > From: Eric Blake [mailto:ebl...@redhat.com] > Sent: Thursday, February 28, 2013 1:02 AM > Cc: Yin Olivia-R63875; libvir-list@redhat.com > Subject: Re: [libvirt] [PATCH v3 2/4] qemu: add dtb capability > > On 02/27/2013 09:56 AM, Eric Blake wrote: > > >> @@ -944,6 +945,8 @@ virQEMUCapsComputeCmdFlags(const char *help, > >> } > >> if (strstr(help, "-uuid")) > >> virQEMUCapsSet(qemuCaps, QEMU_CAPS_UUID); > >> +if (strstr(help, "-dtb")) > >> +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB); > > > > This won't work. -dtb did not exist prior to qemu 1.2, and for all > > qemu newer than 1.2, we do NOT scrape '-help' output; instead, we use > > QMP query commands. You need to figure out the corresponding QMP > > command that we can use to tell when dtb support exists in qemu. > > I stand (slightly) corrected: it looks like -dtb was added in qemu 1.1 > (qemu commit 379b5c7), which means we DO need -help scraping for that > version of qemu. Then, since it predates when QMP queries are reliable, > you can get away with blindly setting QEMU_CAPS_DTB when targetting a qemu > new enough to support QMP queries without actually having to query for it. > But it's still something to be fixed before this patch is ready :) > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] qemu: add dtb capability
Hi Eric,
Thanks for your comments.
> -Original Message-
> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Thursday, February 28, 2013 12:57 AM
> To: Yin Olivia-R63875
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH v3 2/4] qemu: add dtb capability
>
> On 02/27/2013 01:28 AM, Olivia Yin wrote:
> > Signed-off-by: Olivia Yin
> > ---
> > src/qemu/qemu_capabilities.c |3 +++
> > src/qemu/qemu_capabilities.h |1 +
> > 2 files changed, 4 insertions(+), 0 deletions(-)
>
> [not a patch review, but a general comment]
>
> You did deep threading:
>
> 0/4
> \> 1/4
>\> 2/4
> \> 3/4
> \> 4/4
>
> But typically we prefer shallow threading:
>
> 0/4
> +> 1/4
> +> 2/4
> +> 3/4
> \> 4/4
>
> You may want to investigate the settings you are using with git format-
> patch and/or send-email.
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -210,6 +210,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
> >
> >"rng-random", /* 130 */
> >"rng-egd",
> > + "dtb",
>
> OK, so I lied - I'm also doing a patch review:
> This uses a TAB, which is forbidden by 'make syntax-check'.
I'll fix it in next version.
>
> > );
> >
> > struct _virQEMUCaps {
> > @@ -944,6 +945,8 @@ virQEMUCapsComputeCmdFlags(const char *help,
> > }
> > if (strstr(help, "-uuid"))
> > virQEMUCapsSet(qemuCaps, QEMU_CAPS_UUID);
> > +if (strstr(help, "-dtb"))
> > +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB);
>
> This won't work. -dtb did not exist prior to qemu 1.2, and for all qemu
> newer than 1.2, we do NOT scrape '-help' output; instead, we use QMP query
> commands. You need to figure out the corresponding QMP command that we can
> use to tell when dtb support exists in qemu.
How about add QEMU_CAPS_DTB into virQEMUCapsInitQMPBasic?
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40022c1..a5bda24 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -210,6 +210,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"rng-random", /* 130 */
"rng-egd",
+ "dtb",
);
struct _virQEMUCaps {
@@ -1177,6 +1178,9 @@ virQEMUCapsComputeCmdFlags(const char *help,
if (version >= 1002000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+
+if (version >= 11000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB);
return 0;
}
@@ -2294,6 +2298,7 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE);
virQEMUCapsSet(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX);
virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB);
}
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] qemu: add dtb capability
On 02/27/2013 09:56 AM, Eric Blake wrote: >> @@ -944,6 +945,8 @@ virQEMUCapsComputeCmdFlags(const char *help, >> } >> if (strstr(help, "-uuid")) >> virQEMUCapsSet(qemuCaps, QEMU_CAPS_UUID); >> +if (strstr(help, "-dtb")) >> +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB); > > This won't work. -dtb did not exist prior to qemu 1.2, and for all qemu > newer than 1.2, we do NOT scrape '-help' output; instead, we use QMP > query commands. You need to figure out the corresponding QMP command > that we can use to tell when dtb support exists in qemu. I stand (slightly) corrected: it looks like -dtb was added in qemu 1.1 (qemu commit 379b5c7), which means we DO need -help scraping for that version of qemu. Then, since it predates when QMP queries are reliable, you can get away with blindly setting QEMU_CAPS_DTB when targetting a qemu new enough to support QMP queries without actually having to query for it. But it's still something to be fixed before this patch is ready :) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] qemu: add dtb capability
On 02/27/2013 01:28 AM, Olivia Yin wrote:
> Signed-off-by: Olivia Yin
> ---
> src/qemu/qemu_capabilities.c |3 +++
> src/qemu/qemu_capabilities.h |1 +
> 2 files changed, 4 insertions(+), 0 deletions(-)
[not a patch review, but a general comment]
You did deep threading:
0/4
\> 1/4
\> 2/4
\> 3/4
\> 4/4
But typically we prefer shallow threading:
0/4
+> 1/4
+> 2/4
+> 3/4
\> 4/4
You may want to investigate the settings you are using with git
format-patch and/or send-email.
> +++ b/src/qemu/qemu_capabilities.c
> @@ -210,6 +210,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>
>"rng-random", /* 130 */
>"rng-egd",
> + "dtb",
OK, so I lied - I'm also doing a patch review:
This uses a TAB, which is forbidden by 'make syntax-check'.
> );
>
> struct _virQEMUCaps {
> @@ -944,6 +945,8 @@ virQEMUCapsComputeCmdFlags(const char *help,
> }
> if (strstr(help, "-uuid"))
> virQEMUCapsSet(qemuCaps, QEMU_CAPS_UUID);
> +if (strstr(help, "-dtb"))
> +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB);
This won't work. -dtb did not exist prior to qemu 1.2, and for all qemu
newer than 1.2, we do NOT scrape '-help' output; instead, we use QMP
query commands. You need to figure out the corresponding QMP command
that we can use to tell when dtb support exists in qemu.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/4] qemu: add dtb capability
Signed-off-by: Olivia Yin
---
src/qemu/qemu_capabilities.c |3 +++
src/qemu/qemu_capabilities.h |1 +
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40022c1..f6a6ab7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -210,6 +210,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"rng-random", /* 130 */
"rng-egd",
+ "dtb",
);
struct _virQEMUCaps {
@@ -944,6 +945,8 @@ virQEMUCapsComputeCmdFlags(const char *help,
}
if (strstr(help, "-uuid"))
virQEMUCapsSet(qemuCaps, QEMU_CAPS_UUID);
+if (strstr(help, "-dtb"))
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB);
if (strstr(help, "-xen-domid"))
virQEMUCapsSet(qemuCaps, QEMU_CAPS_XEN_DOMID);
else if (strstr(help, "-domid"))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index a895867..f373285 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -171,6 +171,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_OBJECT_RNG_RANDOM = 130, /* the rng-random backend for
virtio rng */
QEMU_CAPS_OBJECT_RNG_EGD = 131, /* EGD protocol daemon for rng */
+QEMU_CAPS_DTB= 132, /* -dtb available */
QEMU_CAPS_LAST, /* this must always be the last item */
};
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
