| Message ID | 20201021071520.2168877-5-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qga: Fix several guest-get-devices issues | expand |
On 10/21/20 9:15 AM, Markus Armbruster wrote: > Simple unions are simpler than flat unions in the schema, but more > complicated in C and on the QMP wire: there's extra indirection in C > and extra nesting on the wire, both pointless. They should be avoided > in new code. > > GuestDeviceId was recently added for guest-get-devices. Convert it to > a flat union. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > qga/qapi-schema.json | 8 ++++++++ > qga/commands-win32.c | 9 ++++----- > 2 files changed, 12 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Wed, Oct 21, 2020 at 11:16 AM Markus Armbruster <armbru@redhat.com> wrote: > Simple unions are simpler than flat unions in the schema, but more > complicated in C and on the QMP wire: there's extra indirection in C > and extra nesting on the wire, both pointless. They should be avoided > in new code. > > GuestDeviceId was recently added for guest-get-devices. Convert it to > a flat union. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- > qga/qapi-schema.json | 8 ++++++++ > qga/commands-win32.c | 9 ++++----- > 2 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index c7bfb8bf6a..fe10631e4c 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -1256,6 +1256,12 @@ > { 'command': 'guest-get-osinfo', > 'returns': 'GuestOSInfo' } > > +## > +# @GuestDeviceType: > +## > +{ 'enum': 'GuestDeviceType', > + 'data': [ 'pci' ] } > + > ## > # @GuestDeviceIdPCI: > # > @@ -1276,6 +1282,8 @@ > # Since: 5.2 > ## > { 'union': 'GuestDeviceId', > + 'base': { 'type': 'GuestDeviceType' }, > + 'discriminator': 'type', > 'data': { 'pci': 'GuestDeviceIdPCI' } } > > ## > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 1efe3ba076..0c33d48aaa 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -2400,16 +2400,15 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error > **errp) > } > skip = false; > > - id = g_new0(GuestDeviceIdPCI, 1); > vendor_id = g_match_info_fetch(match_info, 1); > device_id = g_match_info_fetch(match_info, 2); > - id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16); > - id->device_id = g_ascii_strtoull(device_id, NULL, 16); > > device->id = g_new0(GuestDeviceId, 1); > device->has_id = true; > - device->id->type = GUEST_DEVICE_ID_KIND_PCI; > - device->id->u.pci.data = id; > + device->id->type = GUEST_DEVICE_TYPE_PCI; > + id = &device->id->u.pci; > + id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16); > + id->device_id = g_ascii_strtoull(device_id, NULL, 16); > > g_match_info_free(match_info); > break; > -- > 2.26.2 > > > -- Marc-André Lureau <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 21, 2020 at 11:16 AM Markus Armbruster <<a href="mailto:armbru@redhat.com">armbru@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Simple unions are simpler than flat unions in the schema, but more<br> complicated in C and on the QMP wire: there's extra indirection in C<br> and extra nesting on the wire, both pointless. They should be avoided<br> in new code.<br> <br> GuestDeviceId was recently added for guest-get-devices. Convert it to<br> a flat union.<br> <br> Signed-off-by: Markus Armbruster <<a href="mailto:armbru@redhat.com" target="_blank">armbru@redhat.com</a>><br></blockquote><div><br></div><div>Reviewed-by: Marc-André Lureau <<a href="mailto:marcandre.lureau@redhat.com">marcandre.lureau@redhat.com</a>></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> ---<br> qga/qapi-schema.json | 8 ++++++++<br> qga/commands-win32.c | 9 ++++-----<br> 2 files changed, 12 insertions(+), 5 deletions(-)<br> <br> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json<br> index c7bfb8bf6a..fe10631e4c 100644<br> --- a/qga/qapi-schema.json<br> +++ b/qga/qapi-schema.json<br> @@ -1256,6 +1256,12 @@<br> { 'command': 'guest-get-osinfo',<br> 'returns': 'GuestOSInfo' }<br> <br> +##<br> +# @GuestDeviceType:<br> +##<br> +{ 'enum': 'GuestDeviceType',<br> + 'data': [ 'pci' ] }<br> +<br> ##<br> # @GuestDeviceIdPCI:<br> #<br> @@ -1276,6 +1282,8 @@<br> # Since: 5.2<br> ##<br> { 'union': 'GuestDeviceId',<br> + 'base': { 'type': 'GuestDeviceType' },<br> + 'discriminator': 'type',<br> 'data': { 'pci': 'GuestDeviceIdPCI' } }<br> <br> ##<br> diff --git a/qga/commands-win32.c b/qga/commands-win32.c<br> index 1efe3ba076..0c33d48aaa 100644<br> --- a/qga/commands-win32.c<br> +++ b/qga/commands-win32.c<br> @@ -2400,16 +2400,15 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)<br> }<br> skip = false;<br> <br> - id = g_new0(GuestDeviceIdPCI, 1);<br> vendor_id = g_match_info_fetch(match_info, 1);<br> device_id = g_match_info_fetch(match_info, 2);<br> - id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);<br> - id->device_id = g_ascii_strtoull(device_id, NULL, 16);<br> <br> device->id = g_new0(GuestDeviceId, 1);<br> device->has_id = true;<br> - device->id->type = GUEST_DEVICE_ID_KIND_PCI;<br> - device->id->u.pci.data = id;<br> + device->id->type = GUEST_DEVICE_TYPE_PCI;<br> + id = &device->id->u.pci;<br> + id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);<br> + id->device_id = g_ascii_strtoull(device_id, NULL, 16);<br> <br> g_match_info_free(match_info);<br> break;<br> -- <br> 2.26.2<br> <br> <br> </blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature">Marc-André Lureau<br></div></div>
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index c7bfb8bf6a..fe10631e4c 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -1256,6 +1256,12 @@ { 'command': 'guest-get-osinfo', 'returns': 'GuestOSInfo' } +## +# @GuestDeviceType: +## +{ 'enum': 'GuestDeviceType', + 'data': [ 'pci' ] } + ## # @GuestDeviceIdPCI: # @@ -1276,6 +1282,8 @@ # Since: 5.2 ## { 'union': 'GuestDeviceId', + 'base': { 'type': 'GuestDeviceType' }, + 'discriminator': 'type', 'data': { 'pci': 'GuestDeviceIdPCI' } } ## diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 1efe3ba076..0c33d48aaa 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -2400,16 +2400,15 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp) } skip = false; - id = g_new0(GuestDeviceIdPCI, 1); vendor_id = g_match_info_fetch(match_info, 1); device_id = g_match_info_fetch(match_info, 2); - id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16); - id->device_id = g_ascii_strtoull(device_id, NULL, 16); device->id = g_new0(GuestDeviceId, 1); device->has_id = true; - device->id->type = GUEST_DEVICE_ID_KIND_PCI; - device->id->u.pci.data = id; + device->id->type = GUEST_DEVICE_TYPE_PCI; + id = &device->id->u.pci; + id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16); + id->device_id = g_ascii_strtoull(device_id, NULL, 16); g_match_info_free(match_info); break;
Simple unions are simpler than flat unions in the schema, but more complicated in C and on the QMP wire: there's extra indirection in C and extra nesting on the wire, both pointless. They should be avoided in new code. GuestDeviceId was recently added for guest-get-devices. Convert it to a flat union. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qga/qapi-schema.json | 8 ++++++++ qga/commands-win32.c | 9 ++++----- 2 files changed, 12 insertions(+), 5 deletions(-)