| Message ID | 20201021071520.2168877-3-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qga: Fix several guest-get-devices issues | expand |
Hi On Wed, Oct 21, 2020 at 11:18 AM Markus Armbruster <armbru@redhat.com> wrote: > guest-get-devices returns 'driver-date' as string in the format > YYYY-MM-DD. Goes back to recent commit 2e4211cee4 "qga: add command > guest-get-devices for reporting VirtIO devices". > > We should avoid use of multiple encodings for the same kind of data. > Especially string encodings. Change it to return nanoseconds since > the epoch, like guest-get-time does. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > I think I assumed it was free form human-friendly string, but it has a strict YYYY-MM-DD format in the doc, so let's use the existing one instead. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > qga/qapi-schema.json | 4 ++-- > qga/commands-win32.c | 19 +++++++++++-------- > 2 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index f2c81cda2b..c7bfb8bf6a 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -1282,7 +1282,7 @@ > # @GuestDeviceInfo: > # > # @driver-name: name of the associated driver > -# @driver-date: driver release date in format YYYY-MM-DD > +# @driver-date: driver release date, in nanoseconds since the epoch > # @driver-version: driver version > # @id: device ID > # > @@ -1291,7 +1291,7 @@ > { 'struct': 'GuestDeviceInfo', > 'data': { > 'driver-name': 'str', > - '*driver-date': 'str', > + '*driver-date': 'int', > '*driver-version': 'str', > '*id': 'GuestDeviceId' > } } > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 879b02b6c3..b01616a992 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -1641,6 +1641,12 @@ out: > return head; > } > > +static int64_t filetime_to_ns(const FILETIME *tf) > +{ > + return ((((int64_t)tf->dwHighDateTime << 32) | tf->dwLowDateTime) > + - W32_FT_OFFSET) * 100; > +} > + > int64_t qmp_guest_get_time(Error **errp) > { > SYSTEMTIME ts = {0}; > @@ -1657,8 +1663,7 @@ int64_t qmp_guest_get_time(Error **errp) > return -1; > } > > - return ((((int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime) > - - W32_FT_OFFSET) * 100; > + return filetime_to_ns(&tf); > } > > void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) > @@ -2363,7 +2368,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error > **errp) > slog("enumerating devices"); > for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) { > bool skip = true; > - SYSTEMTIME utc_date; > g_autofree LPWSTR name = NULL; > g_autofree LPFILETIME date = NULL; > g_autofree LPWSTR version = NULL; > @@ -2434,13 +2438,12 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error > **errp) > slog("failed to get driver date"); > continue; > } > - FileTimeToSystemTime(date, &utc_date); > - device->driver_date = g_strdup_printf("%04d-%02d-%02d", > - utc_date.wYear, utc_date.wMonth, utc_date.wDay); > + device->driver_date = filetime_to_ns(date); > device->has_driver_date = true; > > - slog("driver: %s\ndriver version: %s,%s\n", device->driver_name, > - device->driver_date, device->driver_version); > + slog("driver: %s\ndriver version: %" PRId64 ",%s\n", > + device->driver_name, device->driver_date, > + device->driver_version); > item = g_new0(GuestDeviceInfoList, 1); > item->value = g_steal_pointer(&device); > if (!cur_item) { > -- > 2.26.2 > > > -- Marc-André Lureau <div dir="ltr"><div>Hi<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 21, 2020 at 11:18 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">guest-get-devices returns 'driver-date' as string in the format<br> YYYY-MM-DD. Goes back to recent commit 2e4211cee4 "qga: add command<br> guest-get-devices for reporting VirtIO devices".<br> <br> We should avoid use of multiple encodings for the same kind of data.<br> Especially string encodings. Change it to return nanoseconds since<br> the epoch, like guest-get-time does.<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>I think I assumed it was free form human-friendly string, but it has a strict YYYY-MM-DD format in the doc, so let's use the existing one instead.<br></div><div><br></div><div><div>Reviewed-by: Marc-André Lureau <<a href="mailto:marcandre.lureau@redhat.com">marcandre.lureau@redhat.com</a>></div><div> <br></div> </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 | 4 ++--<br> qga/commands-win32.c | 19 +++++++++++--------<br> 2 files changed, 13 insertions(+), 10 deletions(-)<br> <br> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json<br> index f2c81cda2b..c7bfb8bf6a 100644<br> --- a/qga/qapi-schema.json<br> +++ b/qga/qapi-schema.json<br> @@ -1282,7 +1282,7 @@<br> # @GuestDeviceInfo:<br> #<br> # @driver-name: name of the associated driver<br> -# @driver-date: driver release date in format YYYY-MM-DD<br> +# @driver-date: driver release date, in nanoseconds since the epoch<br> # @driver-version: driver version<br> # @id: device ID<br> #<br> @@ -1291,7 +1291,7 @@<br> { 'struct': 'GuestDeviceInfo',<br> 'data': {<br> 'driver-name': 'str',<br> - '*driver-date': 'str',<br> + '*driver-date': 'int',<br> '*driver-version': 'str',<br> '*id': 'GuestDeviceId'<br> } }<br> diff --git a/qga/commands-win32.c b/qga/commands-win32.c<br> index 879b02b6c3..b01616a992 100644<br> --- a/qga/commands-win32.c<br> +++ b/qga/commands-win32.c<br> @@ -1641,6 +1641,12 @@ out:<br> return head;<br> }<br> <br> +static int64_t filetime_to_ns(const FILETIME *tf)<br> +{<br> + return ((((int64_t)tf->dwHighDateTime << 32) | tf->dwLowDateTime)<br> + - W32_FT_OFFSET) * 100;<br> +}<br> +<br> int64_t qmp_guest_get_time(Error **errp)<br> {<br> SYSTEMTIME ts = {0};<br> @@ -1657,8 +1663,7 @@ int64_t qmp_guest_get_time(Error **errp)<br> return -1;<br> }<br> <br> - return ((((int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime)<br> - - W32_FT_OFFSET) * 100;<br> + return filetime_to_ns(&tf);<br> }<br> <br> void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)<br> @@ -2363,7 +2368,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)<br> slog("enumerating devices");<br> for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {<br> bool skip = true;<br> - SYSTEMTIME utc_date;<br> g_autofree LPWSTR name = NULL;<br> g_autofree LPFILETIME date = NULL;<br> g_autofree LPWSTR version = NULL;<br> @@ -2434,13 +2438,12 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)<br> slog("failed to get driver date");<br> continue;<br> }<br> - FileTimeToSystemTime(date, &utc_date);<br> - device->driver_date = g_strdup_printf("%04d-%02d-%02d",<br> - utc_date.wYear, utc_date.wMonth, utc_date.wDay);<br> + device->driver_date = filetime_to_ns(date);<br> device->has_driver_date = true;<br> <br> - slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,<br> - device->driver_date, device->driver_version);<br> + slog("driver: %s\ndriver version: %" PRId64 ",%s\n",<br> + device->driver_name, device->driver_date,<br> + device->driver_version);<br> item = g_new0(GuestDeviceInfoList, 1);<br> item->value = g_steal_pointer(&device);<br> if (!cur_item) {<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 f2c81cda2b..c7bfb8bf6a 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -1282,7 +1282,7 @@ # @GuestDeviceInfo: # # @driver-name: name of the associated driver -# @driver-date: driver release date in format YYYY-MM-DD +# @driver-date: driver release date, in nanoseconds since the epoch # @driver-version: driver version # @id: device ID # @@ -1291,7 +1291,7 @@ { 'struct': 'GuestDeviceInfo', 'data': { 'driver-name': 'str', - '*driver-date': 'str', + '*driver-date': 'int', '*driver-version': 'str', '*id': 'GuestDeviceId' } } diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 879b02b6c3..b01616a992 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -1641,6 +1641,12 @@ out: return head; } +static int64_t filetime_to_ns(const FILETIME *tf) +{ + return ((((int64_t)tf->dwHighDateTime << 32) | tf->dwLowDateTime) + - W32_FT_OFFSET) * 100; +} + int64_t qmp_guest_get_time(Error **errp) { SYSTEMTIME ts = {0}; @@ -1657,8 +1663,7 @@ int64_t qmp_guest_get_time(Error **errp) return -1; } - return ((((int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime) - - W32_FT_OFFSET) * 100; + return filetime_to_ns(&tf); } void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) @@ -2363,7 +2368,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp) slog("enumerating devices"); for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) { bool skip = true; - SYSTEMTIME utc_date; g_autofree LPWSTR name = NULL; g_autofree LPFILETIME date = NULL; g_autofree LPWSTR version = NULL; @@ -2434,13 +2438,12 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp) slog("failed to get driver date"); continue; } - FileTimeToSystemTime(date, &utc_date); - device->driver_date = g_strdup_printf("%04d-%02d-%02d", - utc_date.wYear, utc_date.wMonth, utc_date.wDay); + device->driver_date = filetime_to_ns(date); device->has_driver_date = true; - slog("driver: %s\ndriver version: %s,%s\n", device->driver_name, - device->driver_date, device->driver_version); + slog("driver: %s\ndriver version: %" PRId64 ",%s\n", + device->driver_name, device->driver_date, + device->driver_version); item = g_new0(GuestDeviceInfoList, 1); item->value = g_steal_pointer(&device); if (!cur_item) {
guest-get-devices returns 'driver-date' as string in the format YYYY-MM-DD. Goes back to recent commit 2e4211cee4 "qga: add command guest-get-devices for reporting VirtIO devices". We should avoid use of multiple encodings for the same kind of data. Especially string encodings. Change it to return nanoseconds since the epoch, like guest-get-time does. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qga/qapi-schema.json | 4 ++-- qga/commands-win32.c | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-)