* [PATCH v3] qga: fence guest-set-time if hwclock not available @ 2019-12-05 11:53 Cornelia Huck 2019-12-05 13:05 ` Philippe Mathieu-Daudé 2019-12-10 17:07 ` Cornelia Huck 0 siblings, 2 replies; 12+ messages in thread From: Cornelia Huck @ 2019-12-05 11:53 UTC (permalink / raw) To: Michael Roth Cc: Laszlo Ersek, Cornelia Huck, Daniel P . Berrangé, qemu-devel The Posix implementation of guest-set-time invokes hwclock to set/retrieve the time to/from the hardware clock. If hwclock is not available, the user is currently informed that "hwclock failed to set hardware clock to system time", which is quite misleading. This may happen e.g. on s390x, which has a different timekeeping concept anyway. Let's check for the availability of the hwclock command and return QERR_UNSUPPORTED for guest-set-time if it is not available. Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- v2->v3: - added 'static' keyword to hwclock_path Not sure what tree this is going through; if there's no better place, I can also take this through the s390 tree. --- qga/commands-posix.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 1c1a165daed8..0be301a4ea77 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) pid_t pid; Error *local_err = NULL; struct timeval tv; + static const char hwclock_path[] = "/sbin/hwclock"; + static int hwclock_available = -1; + + if (hwclock_available < 0) { + hwclock_available = (access(hwclock_path, X_OK) == 0); + } + + if (!hwclock_available) { + error_setg(errp, QERR_UNSUPPORTED); + return; + } /* If user has passed a time, validate and set it. */ if (has_time) { @@ -195,7 +206,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) /* Use '/sbin/hwclock -w' to set RTC from the system time, * or '/sbin/hwclock -s' to set the system time from RTC. */ - execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s", + execle(hwclock_path, "hwclock", has_time ? "-w" : "-s", NULL, environ); _exit(EXIT_FAILURE); } else if (pid < 0) { -- 2.21.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 11:53 [PATCH v3] qga: fence guest-set-time if hwclock not available Cornelia Huck @ 2019-12-05 13:05 ` Philippe Mathieu-Daudé 2019-12-05 13:12 ` Cornelia Huck 2019-12-05 15:24 ` Laszlo Ersek 2019-12-10 17:07 ` Cornelia Huck 1 sibling, 2 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2019-12-05 13:05 UTC (permalink / raw) To: Cornelia Huck, Michael Roth Cc: Daniel P . Berrangé, Laszlo Ersek, qemu-devel Hi Cornelia, On 12/5/19 12:53 PM, Cornelia Huck wrote: > The Posix implementation of guest-set-time invokes hwclock to > set/retrieve the time to/from the hardware clock. If hwclock > is not available, the user is currently informed that "hwclock > failed to set hardware clock to system time", which is quite > misleading. This may happen e.g. on s390x, which has a different > timekeeping concept anyway. > > Let's check for the availability of the hwclock command and > return QERR_UNSUPPORTED for guest-set-time if it is not available. > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > > v2->v3: > - added 'static' keyword to hwclock_path > > Not sure what tree this is going through; if there's no better place, > I can also take this through the s390 tree. s390 or trivial trees seems appropriate. > > --- > qga/commands-posix.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 1c1a165daed8..0be301a4ea77 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) > pid_t pid; > Error *local_err = NULL; > struct timeval tv; > + static const char hwclock_path[] = "/sbin/hwclock"; > + static int hwclock_available = -1; > + > + if (hwclock_available < 0) { > + hwclock_available = (access(hwclock_path, X_OK) == 0); > + } > + > + if (!hwclock_available) { > + error_setg(errp, QERR_UNSUPPORTED); In include/qapi/qmp/qerror.h we have: /* * These macros will go away, please don't use in new code, and do not * add new ones! */ Maybe we can replace it by "this feature is not supported on this architecture"? (or without 'on this architecture'). > + return; > + } > > /* If user has passed a time, validate and set it. */ > if (has_time) { > @@ -195,7 +206,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) > > /* Use '/sbin/hwclock -w' to set RTC from the system time, > * or '/sbin/hwclock -s' to set the system time from RTC. */ > - execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s", > + execle(hwclock_path, "hwclock", has_time ? "-w" : "-s", > NULL, environ); > _exit(EXIT_FAILURE); > } else if (pid < 0) { > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 13:05 ` Philippe Mathieu-Daudé @ 2019-12-05 13:12 ` Cornelia Huck 2019-12-05 13:20 ` Philippe Mathieu-Daudé ` (2 more replies) 2019-12-05 15:24 ` Laszlo Ersek 1 sibling, 3 replies; 12+ messages in thread From: Cornelia Huck @ 2019-12-05 13:12 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Daniel P . Berrangé, Laszlo Ersek, Michael Roth, qemu-devel On Thu, 5 Dec 2019 14:05:19 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Cornelia, > > On 12/5/19 12:53 PM, Cornelia Huck wrote: > > The Posix implementation of guest-set-time invokes hwclock to > > set/retrieve the time to/from the hardware clock. If hwclock > > is not available, the user is currently informed that "hwclock > > failed to set hardware clock to system time", which is quite > > misleading. This may happen e.g. on s390x, which has a different > > timekeeping concept anyway. > > > > Let's check for the availability of the hwclock command and > > return QERR_UNSUPPORTED for guest-set-time if it is not available. > > > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > > > v2->v3: > > - added 'static' keyword to hwclock_path > > > > Not sure what tree this is going through; if there's no better place, > > I can also take this through the s390 tree. > > s390 or trivial trees seems appropriate. > > > > > --- > > qga/commands-posix.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > > index 1c1a165daed8..0be301a4ea77 100644 > > --- a/qga/commands-posix.c > > +++ b/qga/commands-posix.c > > @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) > > pid_t pid; > > Error *local_err = NULL; > > struct timeval tv; > > + static const char hwclock_path[] = "/sbin/hwclock"; > > + static int hwclock_available = -1; > > + > > + if (hwclock_available < 0) { > > + hwclock_available = (access(hwclock_path, X_OK) == 0); > > + } > > + > > + if (!hwclock_available) { > > + error_setg(errp, QERR_UNSUPPORTED); > > In include/qapi/qmp/qerror.h we have: > > /* > * These macros will go away, please don't use in new code, and do not > * add new ones! > */ Sigh, it is really hard to keep track here :( I just copied from other callers in this file... > > Maybe we can replace it by "this feature is not supported on this > architecture"? (or without 'on this architecture'). This is not really architecture specific, you'd get this on any setup that does not have /sbin/hwclock. Q: Is libvirt doing anything with such an error message from QEMU? Do we have the freedom to say e.g "guest-set-time is not supported" or so? Or is it beneficial to print the same error message for any unsupported feature? > > > + return; > > + } > > > > /* If user has passed a time, validate and set it. */ > > if (has_time) { > > @@ -195,7 +206,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) > > > > /* Use '/sbin/hwclock -w' to set RTC from the system time, > > * or '/sbin/hwclock -s' to set the system time from RTC. */ > > - execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s", > > + execle(hwclock_path, "hwclock", has_time ? "-w" : "-s", > > NULL, environ); > > _exit(EXIT_FAILURE); > > } else if (pid < 0) { > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 13:12 ` Cornelia Huck @ 2019-12-05 13:20 ` Philippe Mathieu-Daudé 2019-12-05 14:21 ` Michal Privoznik 2019-12-06 7:17 ` Markus Armbruster 2 siblings, 0 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2019-12-05 13:20 UTC (permalink / raw) To: Cornelia Huck Cc: qemu-devel, Daniel P . Berrangé, libvir-list, Michael Roth, Markus Armbruster, Laszlo Ersek On 12/5/19 2:12 PM, Cornelia Huck wrote: > On Thu, 5 Dec 2019 14:05:19 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> Hi Cornelia, >> >> On 12/5/19 12:53 PM, Cornelia Huck wrote: >>> The Posix implementation of guest-set-time invokes hwclock to >>> set/retrieve the time to/from the hardware clock. If hwclock >>> is not available, the user is currently informed that "hwclock >>> failed to set hardware clock to system time", which is quite >>> misleading. This may happen e.g. on s390x, which has a different >>> timekeeping concept anyway. >>> >>> Let's check for the availability of the hwclock command and >>> return QERR_UNSUPPORTED for guest-set-time if it is not available. >>> >>> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >>> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>> --- >>> >>> v2->v3: >>> - added 'static' keyword to hwclock_path >>> >>> Not sure what tree this is going through; if there's no better place, >>> I can also take this through the s390 tree. >> >> s390 or trivial trees seems appropriate. >> >>> >>> --- >>> qga/commands-posix.c | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >>> index 1c1a165daed8..0be301a4ea77 100644 >>> --- a/qga/commands-posix.c >>> +++ b/qga/commands-posix.c >>> @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) >>> pid_t pid; >>> Error *local_err = NULL; >>> struct timeval tv; >>> + static const char hwclock_path[] = "/sbin/hwclock"; >>> + static int hwclock_available = -1; >>> + >>> + if (hwclock_available < 0) { >>> + hwclock_available = (access(hwclock_path, X_OK) == 0); >>> + } >>> + >>> + if (!hwclock_available) { >>> + error_setg(errp, QERR_UNSUPPORTED); >> >> In include/qapi/qmp/qerror.h we have: >> >> /* >> * These macros will go away, please don't use in new code, and do not >> * add new ones! >> */ > > Sigh, it is really hard to keep track here :( I just copied from other > callers in this file... > >> >> Maybe we can replace it by "this feature is not supported on this >> architecture"? (or without 'on this architecture'). > > This is not really architecture specific, you'd get this on any setup > that does not have /sbin/hwclock. > > Q: Is libvirt doing anything with such an error message from QEMU? Do > we have the freedom to say e.g "guest-set-time is not supported" or so? > Or is it beneficial to print the same error message for any unsupported > feature? Cc'ing Markus who added the command and libvir-list. Using "guest-set-time is not supported" or "this command is not supported": Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> + return; >>> + } >>> >>> /* If user has passed a time, validate and set it. */ >>> if (has_time) { >>> @@ -195,7 +206,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) >>> >>> /* Use '/sbin/hwclock -w' to set RTC from the system time, >>> * or '/sbin/hwclock -s' to set the system time from RTC. */ >>> - execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s", >>> + execle(hwclock_path, "hwclock", has_time ? "-w" : "-s", >>> NULL, environ); >>> _exit(EXIT_FAILURE); >>> } else if (pid < 0) { >>> >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 13:12 ` Cornelia Huck 2019-12-05 13:20 ` Philippe Mathieu-Daudé @ 2019-12-05 14:21 ` Michal Privoznik 2019-12-06 7:17 ` Markus Armbruster 2 siblings, 0 replies; 12+ messages in thread From: Michal Privoznik @ 2019-12-05 14:21 UTC (permalink / raw) To: Cornelia Huck, Philippe Mathieu-Daudé Cc: Laszlo Ersek, Daniel P . Berrangé, Michael Roth, qemu-devel On 12/5/19 2:12 PM, Cornelia Huck wrote: > On Thu, 5 Dec 2019 14:05:19 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> Hi Cornelia, >> >> On 12/5/19 12:53 PM, Cornelia Huck wrote: >>> The Posix implementation of guest-set-time invokes hwclock to >>> set/retrieve the time to/from the hardware clock. If hwclock >>> is not available, the user is currently informed that "hwclock >>> failed to set hardware clock to system time", which is quite >>> misleading. This may happen e.g. on s390x, which has a different >>> timekeeping concept anyway. >>> >>> Let's check for the availability of the hwclock command and >>> return QERR_UNSUPPORTED for guest-set-time if it is not available. >>> >>> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >>> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>> --- >>> >>> v2->v3: >>> - added 'static' keyword to hwclock_path >>> >>> Not sure what tree this is going through; if there's no better place, >>> I can also take this through the s390 tree. >> >> s390 or trivial trees seems appropriate. >> >>> >>> --- >>> qga/commands-posix.c | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >>> index 1c1a165daed8..0be301a4ea77 100644 >>> --- a/qga/commands-posix.c >>> +++ b/qga/commands-posix.c >>> @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) >>> pid_t pid; >>> Error *local_err = NULL; >>> struct timeval tv; >>> + static const char hwclock_path[] = "/sbin/hwclock"; >>> + static int hwclock_available = -1; >>> + >>> + if (hwclock_available < 0) { >>> + hwclock_available = (access(hwclock_path, X_OK) == 0); >>> + } >>> + >>> + if (!hwclock_available) { >>> + error_setg(errp, QERR_UNSUPPORTED); >> >> In include/qapi/qmp/qerror.h we have: >> >> /* >> * These macros will go away, please don't use in new code, and do not >> * add new ones! >> */ > > Sigh, it is really hard to keep track here :( I just copied from other > callers in this file... > >> >> Maybe we can replace it by "this feature is not supported on this >> architecture"? (or without 'on this architecture'). > > This is not really architecture specific, you'd get this on any setup > that does not have /sbin/hwclock. > > Q: Is libvirt doing anything with such an error message from QEMU? Do > we have the freedom to say e.g "guest-set-time is not supported" or so? > Or is it beneficial to print the same error message for any unsupported > feature? No. Libvirt threats error messages as an opaque data. In a few cases we check for the class of the error and for instance issue a different command if class was CommandNotFound. You are free to change error messages as you wish. Note that this is not true for HMP - there libvirt does some parsing to figure out the source of error. For instance: https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_monitor_text.c;h=9054682d608b13347880f36cacd0e023151322e6;hb=HEAD#l36 Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 13:12 ` Cornelia Huck 2019-12-05 13:20 ` Philippe Mathieu-Daudé 2019-12-05 14:21 ` Michal Privoznik @ 2019-12-06 7:17 ` Markus Armbruster 2019-12-09 18:33 ` Cornelia Huck 2 siblings, 1 reply; 12+ messages in thread From: Markus Armbruster @ 2019-12-06 7:17 UTC (permalink / raw) To: Cornelia Huck Cc: qemu-devel, Daniel P . Berrangé, Philippe Mathieu-Daudé, Michael Roth, Laszlo Ersek Cornelia Huck <cohuck@redhat.com> writes: > On Thu, 5 Dec 2019 14:05:19 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> Hi Cornelia, >> >> On 12/5/19 12:53 PM, Cornelia Huck wrote: >> > The Posix implementation of guest-set-time invokes hwclock to >> > set/retrieve the time to/from the hardware clock. If hwclock >> > is not available, the user is currently informed that "hwclock >> > failed to set hardware clock to system time", which is quite >> > misleading. This may happen e.g. on s390x, which has a different >> > timekeeping concept anyway. >> > >> > Let's check for the availability of the hwclock command and >> > return QERR_UNSUPPORTED for guest-set-time if it is not available. >> > >> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >> > Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> >> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> > --- >> > >> > v2->v3: >> > - added 'static' keyword to hwclock_path >> > >> > Not sure what tree this is going through; if there's no better place, >> > I can also take this through the s390 tree. >> >> s390 or trivial trees seems appropriate. >> >> > >> > --- >> > qga/commands-posix.c | 13 ++++++++++++- >> > 1 file changed, 12 insertions(+), 1 deletion(-) >> > >> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> > index 1c1a165daed8..0be301a4ea77 100644 >> > --- a/qga/commands-posix.c >> > +++ b/qga/commands-posix.c >> > @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) >> > pid_t pid; >> > Error *local_err = NULL; >> > struct timeval tv; >> > + static const char hwclock_path[] = "/sbin/hwclock"; >> > + static int hwclock_available = -1; >> > + >> > + if (hwclock_available < 0) { >> > + hwclock_available = (access(hwclock_path, X_OK) == 0); >> > + } >> > + >> > + if (!hwclock_available) { >> > + error_setg(errp, QERR_UNSUPPORTED); >> >> In include/qapi/qmp/qerror.h we have: >> >> /* >> * These macros will go away, please don't use in new code, and do not >> * add new ones! >> */ > > Sigh, it is really hard to keep track here :( I just copied from other > callers in this file... I'm not faulting you for that. I think this new use is acceptable. For details, see my other reply in this thread. [...] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-06 7:17 ` Markus Armbruster @ 2019-12-09 18:33 ` Cornelia Huck 2019-12-10 16:38 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 12+ messages in thread From: Cornelia Huck @ 2019-12-09 18:33 UTC (permalink / raw) To: Markus Armbruster, Philippe Mathieu-Daudé Cc: Laszlo Ersek, Daniel P . Berrangé, Michael Roth, qemu-devel On Fri, 06 Dec 2019 08:17:27 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Cornelia Huck <cohuck@redhat.com> writes: > > > On Thu, 5 Dec 2019 14:05:19 +0100 > > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > >> Hi Cornelia, > >> > >> On 12/5/19 12:53 PM, Cornelia Huck wrote: > >> > The Posix implementation of guest-set-time invokes hwclock to > >> > set/retrieve the time to/from the hardware clock. If hwclock > >> > is not available, the user is currently informed that "hwclock > >> > failed to set hardware clock to system time", which is quite > >> > misleading. This may happen e.g. on s390x, which has a different > >> > timekeeping concept anyway. > >> > > >> > Let's check for the availability of the hwclock command and > >> > return QERR_UNSUPPORTED for guest-set-time if it is not available. > >> > > >> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > >> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > >> > Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> > >> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > >> > --- > >> > > >> > v2->v3: > >> > - added 'static' keyword to hwclock_path > >> > > >> > Not sure what tree this is going through; if there's no better place, > >> > I can also take this through the s390 tree. > >> > >> s390 or trivial trees seems appropriate. > >> > >> > > >> > --- > >> > qga/commands-posix.c | 13 ++++++++++++- > >> > 1 file changed, 12 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > >> > index 1c1a165daed8..0be301a4ea77 100644 > >> > --- a/qga/commands-posix.c > >> > +++ b/qga/commands-posix.c > >> > @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) > >> > pid_t pid; > >> > Error *local_err = NULL; > >> > struct timeval tv; > >> > + static const char hwclock_path[] = "/sbin/hwclock"; > >> > + static int hwclock_available = -1; > >> > + > >> > + if (hwclock_available < 0) { > >> > + hwclock_available = (access(hwclock_path, X_OK) == 0); > >> > + } > >> > + > >> > + if (!hwclock_available) { > >> > + error_setg(errp, QERR_UNSUPPORTED); > >> > >> In include/qapi/qmp/qerror.h we have: > >> > >> /* > >> * These macros will go away, please don't use in new code, and do not > >> * add new ones! > >> */ > > > > Sigh, it is really hard to keep track here :( I just copied from other > > callers in this file... > > I'm not faulting you for that. > > I think this new use is acceptable. For details, see my other reply in > this thread. Ok, thanks for your explanation there. I guess I'll queue this on s390-next... Philippe, any objections to adding your R-b to the unmodified patch? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-09 18:33 ` Cornelia Huck @ 2019-12-10 16:38 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2019-12-10 16:38 UTC (permalink / raw) To: Cornelia Huck, Markus Armbruster Cc: Laszlo Ersek, Daniel P . Berrangé, Michael Roth, qemu-devel On 12/9/19 7:33 PM, Cornelia Huck wrote: > On Fri, 06 Dec 2019 08:17:27 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> Cornelia Huck <cohuck@redhat.com> writes: >> >>> On Thu, 5 Dec 2019 14:05:19 +0100 >>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >>> >>>> Hi Cornelia, >>>> >>>> On 12/5/19 12:53 PM, Cornelia Huck wrote: >>>>> The Posix implementation of guest-set-time invokes hwclock to >>>>> set/retrieve the time to/from the hardware clock. If hwclock >>>>> is not available, the user is currently informed that "hwclock >>>>> failed to set hardware clock to system time", which is quite >>>>> misleading. This may happen e.g. on s390x, which has a different >>>>> timekeeping concept anyway. >>>>> >>>>> Let's check for the availability of the hwclock command and >>>>> return QERR_UNSUPPORTED for guest-set-time if it is not available. >>>>> >>>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >>>>> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>>>> --- >>>>> >>>>> v2->v3: >>>>> - added 'static' keyword to hwclock_path >>>>> >>>>> Not sure what tree this is going through; if there's no better place, >>>>> I can also take this through the s390 tree. >>>> >>>> s390 or trivial trees seems appropriate. >>>> >>>>> >>>>> --- >>>>> qga/commands-posix.c | 13 ++++++++++++- >>>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >>>>> index 1c1a165daed8..0be301a4ea77 100644 >>>>> --- a/qga/commands-posix.c >>>>> +++ b/qga/commands-posix.c >>>>> @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) >>>>> pid_t pid; >>>>> Error *local_err = NULL; >>>>> struct timeval tv; >>>>> + static const char hwclock_path[] = "/sbin/hwclock"; >>>>> + static int hwclock_available = -1; >>>>> + >>>>> + if (hwclock_available < 0) { >>>>> + hwclock_available = (access(hwclock_path, X_OK) == 0); >>>>> + } >>>>> + >>>>> + if (!hwclock_available) { >>>>> + error_setg(errp, QERR_UNSUPPORTED); >>>> >>>> In include/qapi/qmp/qerror.h we have: >>>> >>>> /* >>>> * These macros will go away, please don't use in new code, and do not >>>> * add new ones! >>>> */ >>> >>> Sigh, it is really hard to keep track here :( I just copied from other >>> callers in this file... >> >> I'm not faulting you for that. >> >> I think this new use is acceptable. For details, see my other reply in >> this thread. > > Ok, thanks for your explanation there. > > I guess I'll queue this on s390-next... Philippe, any objections to > adding your R-b to the unmodified patch? Certainly, sorry for the delay/noise on this trivial patch, I learned the subtle differences between comments in code and reality :) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 13:05 ` Philippe Mathieu-Daudé 2019-12-05 13:12 ` Cornelia Huck @ 2019-12-05 15:24 ` Laszlo Ersek 2019-12-06 7:15 ` Markus Armbruster 1 sibling, 1 reply; 12+ messages in thread From: Laszlo Ersek @ 2019-12-05 15:24 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Cornelia Huck, Michael Roth Cc: Daniel P . Berrangé, qemu-devel, Markus Armbruster On 12/05/19 14:05, Philippe Mathieu-Daudé wrote: > Hi Cornelia, > > On 12/5/19 12:53 PM, Cornelia Huck wrote: >> The Posix implementation of guest-set-time invokes hwclock to >> set/retrieve the time to/from the hardware clock. If hwclock >> is not available, the user is currently informed that "hwclock >> failed to set hardware clock to system time", which is quite >> misleading. This may happen e.g. on s390x, which has a different >> timekeeping concept anyway. >> >> Let's check for the availability of the hwclock command and >> return QERR_UNSUPPORTED for guest-set-time if it is not available. >> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> --- >> >> v2->v3: >> - added 'static' keyword to hwclock_path >> >> Not sure what tree this is going through; if there's no better place, >> I can also take this through the s390 tree. > > s390 or trivial trees seems appropriate. > >> >> --- >> qga/commands-posix.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> index 1c1a165daed8..0be301a4ea77 100644 >> --- a/qga/commands-posix.c >> +++ b/qga/commands-posix.c >> @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t >> time_ns, Error **errp) >> pid_t pid; >> Error *local_err = NULL; >> struct timeval tv; >> + static const char hwclock_path[] = "/sbin/hwclock"; >> + static int hwclock_available = -1; >> + >> + if (hwclock_available < 0) { >> + hwclock_available = (access(hwclock_path, X_OK) == 0); >> + } >> + >> + if (!hwclock_available) { >> + error_setg(errp, QERR_UNSUPPORTED); > > In include/qapi/qmp/qerror.h we have: > > /* > * These macros will go away, please don't use in new code, and do not > * add new ones! > */ Obviously, the last word on this belongs to Markus (CC'd) -- he added that comment. I'd just like to point out *when* that comment was added: approx. four and half years ago. (See commit 4629ed1e9896.) I've always associated QERR_UNSUPPORTED with QMP interfaces rejecting invocation due to lack of support. I don't think one more instance of QERR_UNSUPPORTED will matter much, when we'll "finally" :) convert or eliminate the other 35! (Yes, I've counted.) In case it's unacceptable to add one more QERR_UNSUPPORTED: what is the official solution that replaces it? I assume it was explained in the series that included commit 4629ed1e9896, but I can't easily tell. (And, there is no "QERR_" match in docs/.) Hmmm, more history digging... In the 4629ed1e9896..v4.2.0-rc4 set of commits, the following commits introduced new instances of QERR_UNSUPPORTED: - e09484efbc9d ("qmp: add QMP interface "query-cpu-model-expansion"", 2016-09-06) - 0031e0d68339 ("qmp: add QMP interface "query-cpu-model-comparison"", 2016-09-06) - b18b6043341d ("qmp: add QMP interface "query-cpu-model-baseline"", 2016-09-06) - 1007a37e2082 ("smbios: filter based on CONFIG_SMBIOS rather than TARGET", 2017-01-16) - 9f57061c3555 ("acpi: filter based on CONFIG_ACPI_X86 rather than TARGET", 2017-01-16) - 39164c136cba ("qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' commands", 2017-03-02) - 161a56a9065f ("qga: Add 'guest-get-users' command", 2017-04-26) - 53c58e64d0a2 ("qga: Add `guest-get-timezone` command", 2017-04-27) - e674605f9821 ("qemu-ga: check if utmpx.h is available on the system", 2017-07-17) I don't claim that all of those additions have stuck with us, to v4.2.0-rc4. Yet, in general, practice doesn't seem to have followed the intended deprecation. > > Maybe we can replace it by "this feature is not supported on this > architecture"? (or without 'on this architecture'). I think if we replace QERR_UNSUPPORTED with anything, it should be "similarly standardized". (Lack of support for a given QMP interface is pretty common, I think.) Thanks, Laszlo > >> + return; >> + } >> /* If user has passed a time, validate and set it. */ >> if (has_time) { >> @@ -195,7 +206,7 @@ void qmp_guest_set_time(bool has_time, int64_t >> time_ns, Error **errp) >> /* Use '/sbin/hwclock -w' to set RTC from the system time, >> * or '/sbin/hwclock -s' to set the system time from RTC. */ >> - execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s", >> + execle(hwclock_path, "hwclock", has_time ? "-w" : "-s", >> NULL, environ); >> _exit(EXIT_FAILURE); >> } else if (pid < 0) { >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 15:24 ` Laszlo Ersek @ 2019-12-06 7:15 ` Markus Armbruster 2019-12-06 9:02 ` Laszlo Ersek 0 siblings, 1 reply; 12+ messages in thread From: Markus Armbruster @ 2019-12-06 7:15 UTC (permalink / raw) To: Laszlo Ersek Cc: Daniel P . Berrangé, Cornelia Huck, Philippe Mathieu-Daudé, Michael Roth, qemu-devel Laszlo Ersek <lersek@redhat.com> writes: > On 12/05/19 14:05, Philippe Mathieu-Daudé wrote: >> Hi Cornelia, >> >> On 12/5/19 12:53 PM, Cornelia Huck wrote: >>> The Posix implementation of guest-set-time invokes hwclock to >>> set/retrieve the time to/from the hardware clock. If hwclock >>> is not available, the user is currently informed that "hwclock >>> failed to set hardware clock to system time", which is quite >>> misleading. This may happen e.g. on s390x, which has a different >>> timekeeping concept anyway. >>> >>> Let's check for the availability of the hwclock command and >>> return QERR_UNSUPPORTED for guest-set-time if it is not available. >>> >>> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >>> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>> --- >>> >>> v2->v3: >>> - added 'static' keyword to hwclock_path >>> >>> Not sure what tree this is going through; if there's no better place, >>> I can also take this through the s390 tree. >> >> s390 or trivial trees seems appropriate. >> >>> >>> --- >>> qga/commands-posix.c | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >>> index 1c1a165daed8..0be301a4ea77 100644 >>> --- a/qga/commands-posix.c >>> +++ b/qga/commands-posix.c >>> @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t >>> time_ns, Error **errp) >>> pid_t pid; >>> Error *local_err = NULL; >>> struct timeval tv; >>> + static const char hwclock_path[] = "/sbin/hwclock"; >>> + static int hwclock_available = -1; >>> + >>> + if (hwclock_available < 0) { >>> + hwclock_available = (access(hwclock_path, X_OK) == 0); >>> + } >>> + >>> + if (!hwclock_available) { >>> + error_setg(errp, QERR_UNSUPPORTED); >> >> In include/qapi/qmp/qerror.h we have: >> >> /* >> * These macros will go away, please don't use in new code, and do not >> * add new ones! >> */ > > Obviously, the last word on this belongs to Markus (CC'd) -- he added > that comment. I'd just like to point out *when* that comment was added: > approx. four and half years ago. (See commit 4629ed1e9896.) Yes, with a big push to finally kill qerror_report(). I was too exhausted to finish the job and kill all the remaining QERR_, too. I'm dead serious on the "do not add new ones" part. On the "don't use in new code" part, I'm quite willing to tolerate reasonable exceptions, e.g. to maintain consistency with old code. This one looks like a reasonable exception to me. > I've always associated QERR_UNSUPPORTED with QMP interfaces rejecting > invocation due to lack of support. I don't think one more instance of > QERR_UNSUPPORTED will matter much, when we'll "finally" :) convert or > eliminate the other 35! (Yes, I've counted.) > > In case it's unacceptable to add one more QERR_UNSUPPORTED: what is the > official solution that replaces it? > > I assume it was explained in the series that included commit > 4629ed1e9896, but I can't easily tell. (And, there is no "QERR_" match > in docs/.) See "exhausted" above. > Hmmm, more history digging... In the 4629ed1e9896..v4.2.0-rc4 set of > commits, the following commits introduced new instances of > QERR_UNSUPPORTED: > > - e09484efbc9d ("qmp: add QMP interface "query-cpu-model-expansion"", 2016-09-06) > - 0031e0d68339 ("qmp: add QMP interface "query-cpu-model-comparison"", 2016-09-06) > - b18b6043341d ("qmp: add QMP interface "query-cpu-model-baseline"", 2016-09-06) > - 1007a37e2082 ("smbios: filter based on CONFIG_SMBIOS rather than TARGET", 2017-01-16) > - 9f57061c3555 ("acpi: filter based on CONFIG_ACPI_X86 rather than TARGET", 2017-01-16) > - 39164c136cba ("qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' commands", 2017-03-02) > - 161a56a9065f ("qga: Add 'guest-get-users' command", 2017-04-26) > - 53c58e64d0a2 ("qga: Add `guest-get-timezone` command", 2017-04-27) > - e674605f9821 ("qemu-ga: check if utmpx.h is available on the system", 2017-07-17) > > I don't claim that all of those additions have stuck with us, to > v4.2.0-rc4. Yet, in general, practice doesn't seem to have followed the > intended deprecation. > >> >> Maybe we can replace it by "this feature is not supported on this >> architecture"? (or without 'on this architecture'). > > I think if we replace QERR_UNSUPPORTED with anything, it should be > "similarly standardized". (Lack of support for a given QMP interface is > pretty common, I think.) Here's my the general idea on getting rid of qerror.h. The QERR_ macros effectively factor out common error message format strings. DRY is a legitimate concern. However, I dislike (1) passing anything but string literals as format to printf()-style function, and (2) tempting people to reuse existing error messages where a new error message would be more helpful. Note that the error_setg() is *also* common. So take DRY to the next level: factor out the common error_setg(errp, "literal error format string", ...) along with whatever error handling is also common in a helper function, and call that. However, do that only where the errors are truly common. Where they're just similar, duplicate the error message, and maybe specialize it for specific error situations. >>> + return; >>> + } >>> /* If user has passed a time, validate and set it. */ >>> if (has_time) { >>> @@ -195,7 +206,7 @@ void qmp_guest_set_time(bool has_time, int64_t >>> time_ns, Error **errp) >>> /* Use '/sbin/hwclock -w' to set RTC from the system time, >>> * or '/sbin/hwclock -s' to set the system time from RTC. */ >>> - execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s", >>> + execle(hwclock_path, "hwclock", has_time ? "-w" : "-s", >>> NULL, environ); >>> _exit(EXIT_FAILURE); >>> } else if (pid < 0) { >>> >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-06 7:15 ` Markus Armbruster @ 2019-12-06 9:02 ` Laszlo Ersek 0 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2019-12-06 9:02 UTC (permalink / raw) To: Markus Armbruster Cc: Daniel P . Berrangé, Cornelia Huck, Philippe Mathieu-Daudé, Michael Roth, qemu-devel On 12/06/19 08:15, Markus Armbruster wrote: > Laszlo Ersek <lersek@redhat.com> writes: > >> On 12/05/19 14:05, Philippe Mathieu-Daudé wrote: >>> Hi Cornelia, >>> >>> On 12/5/19 12:53 PM, Cornelia Huck wrote: >>>> The Posix implementation of guest-set-time invokes hwclock to >>>> set/retrieve the time to/from the hardware clock. If hwclock >>>> is not available, the user is currently informed that "hwclock >>>> failed to set hardware clock to system time", which is quite >>>> misleading. This may happen e.g. on s390x, which has a different >>>> timekeeping concept anyway. >>>> >>>> Let's check for the availability of the hwclock command and >>>> return QERR_UNSUPPORTED for guest-set-time if it is not available. >>>> >>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >>>> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>>> --- >>>> >>>> v2->v3: >>>> - added 'static' keyword to hwclock_path >>>> >>>> Not sure what tree this is going through; if there's no better place, >>>> I can also take this through the s390 tree. >>> >>> s390 or trivial trees seems appropriate. >>> >>>> >>>> --- >>>> qga/commands-posix.c | 13 ++++++++++++- >>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >>>> index 1c1a165daed8..0be301a4ea77 100644 >>>> --- a/qga/commands-posix.c >>>> +++ b/qga/commands-posix.c >>>> @@ -156,6 +156,17 @@ void qmp_guest_set_time(bool has_time, int64_t >>>> time_ns, Error **errp) >>>> pid_t pid; >>>> Error *local_err = NULL; >>>> struct timeval tv; >>>> + static const char hwclock_path[] = "/sbin/hwclock"; >>>> + static int hwclock_available = -1; >>>> + >>>> + if (hwclock_available < 0) { >>>> + hwclock_available = (access(hwclock_path, X_OK) == 0); >>>> + } >>>> + >>>> + if (!hwclock_available) { >>>> + error_setg(errp, QERR_UNSUPPORTED); >>> >>> In include/qapi/qmp/qerror.h we have: >>> >>> /* >>> * These macros will go away, please don't use in new code, and do not >>> * add new ones! >>> */ >> >> Obviously, the last word on this belongs to Markus (CC'd) -- he added >> that comment. I'd just like to point out *when* that comment was added: >> approx. four and half years ago. (See commit 4629ed1e9896.) > > Yes, with a big push to finally kill qerror_report(). I was too > exhausted to finish the job and kill all the remaining QERR_, too. > > I'm dead serious on the "do not add new ones" part. > > On the "don't use in new code" part, I'm quite willing to tolerate > reasonable exceptions, e.g. to maintain consistency with old code. > > This one looks like a reasonable exception to me. > >> I've always associated QERR_UNSUPPORTED with QMP interfaces rejecting >> invocation due to lack of support. I don't think one more instance of >> QERR_UNSUPPORTED will matter much, when we'll "finally" :) convert or >> eliminate the other 35! (Yes, I've counted.) >> >> In case it's unacceptable to add one more QERR_UNSUPPORTED: what is the >> official solution that replaces it? >> >> I assume it was explained in the series that included commit >> 4629ed1e9896, but I can't easily tell. (And, there is no "QERR_" match >> in docs/.) > > See "exhausted" above. > >> Hmmm, more history digging... In the 4629ed1e9896..v4.2.0-rc4 set of >> commits, the following commits introduced new instances of >> QERR_UNSUPPORTED: >> >> - e09484efbc9d ("qmp: add QMP interface "query-cpu-model-expansion"", 2016-09-06) >> - 0031e0d68339 ("qmp: add QMP interface "query-cpu-model-comparison"", 2016-09-06) >> - b18b6043341d ("qmp: add QMP interface "query-cpu-model-baseline"", 2016-09-06) >> - 1007a37e2082 ("smbios: filter based on CONFIG_SMBIOS rather than TARGET", 2017-01-16) >> - 9f57061c3555 ("acpi: filter based on CONFIG_ACPI_X86 rather than TARGET", 2017-01-16) >> - 39164c136cba ("qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' commands", 2017-03-02) >> - 161a56a9065f ("qga: Add 'guest-get-users' command", 2017-04-26) >> - 53c58e64d0a2 ("qga: Add `guest-get-timezone` command", 2017-04-27) >> - e674605f9821 ("qemu-ga: check if utmpx.h is available on the system", 2017-07-17) >> >> I don't claim that all of those additions have stuck with us, to >> v4.2.0-rc4. Yet, in general, practice doesn't seem to have followed the >> intended deprecation. >> >>> >>> Maybe we can replace it by "this feature is not supported on this >>> architecture"? (or without 'on this architecture'). >> >> I think if we replace QERR_UNSUPPORTED with anything, it should be >> "similarly standardized". (Lack of support for a given QMP interface is >> pretty common, I think.) > > Here's my the general idea on getting rid of qerror.h. The QERR_ macros > effectively factor out common error message format strings. DRY is a > legitimate concern. However, I dislike (1) passing anything but string > literals as format to printf()-style function, and (2) tempting people > to reuse existing error messages where a new error message would be more > helpful. > > Note that the error_setg() is *also* common. So take DRY to the next > level: factor out the common error_setg(errp, "literal error format > string", ...) along with whatever error handling is also common in a > helper function, and call that. Good point, I didn't think of (e.g.) error_set_qmp_unsupported(). Thank you for the details! Laszlo > > However, do that only where the errors are truly common. Where they're > just similar, duplicate the error message, and maybe specialize it for > specific error situations. > >>>> + return; >>>> + } >>>> /* If user has passed a time, validate and set it. */ >>>> if (has_time) { >>>> @@ -195,7 +206,7 @@ void qmp_guest_set_time(bool has_time, int64_t >>>> time_ns, Error **errp) >>>> /* Use '/sbin/hwclock -w' to set RTC from the system time, >>>> * or '/sbin/hwclock -s' to set the system time from RTC. */ >>>> - execle("/sbin/hwclock", "hwclock", has_time ? "-w" : "-s", >>>> + execle(hwclock_path, "hwclock", has_time ? "-w" : "-s", >>>> NULL, environ); >>>> _exit(EXIT_FAILURE); >>>> } else if (pid < 0) { >>>> >>> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] qga: fence guest-set-time if hwclock not available 2019-12-05 11:53 [PATCH v3] qga: fence guest-set-time if hwclock not available Cornelia Huck 2019-12-05 13:05 ` Philippe Mathieu-Daudé @ 2019-12-10 17:07 ` Cornelia Huck 1 sibling, 0 replies; 12+ messages in thread From: Cornelia Huck @ 2019-12-10 17:07 UTC (permalink / raw) To: Michael Roth; +Cc: Laszlo Ersek, Daniel P . Berrangé, qemu-devel On Thu, 5 Dec 2019 12:53:50 +0100 Cornelia Huck <cohuck@redhat.com> wrote: > The Posix implementation of guest-set-time invokes hwclock to > set/retrieve the time to/from the hardware clock. If hwclock > is not available, the user is currently informed that "hwclock > failed to set hardware clock to system time", which is quite > misleading. This may happen e.g. on s390x, which has a different > timekeeping concept anyway. > > Let's check for the availability of the hwclock command and > return QERR_UNSUPPORTED for guest-set-time if it is not available. > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > > v2->v3: > - added 'static' keyword to hwclock_path > > Not sure what tree this is going through; if there's no better place, > I can also take this through the s390 tree. > > --- > qga/commands-posix.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) Queued to s390-next. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-12-10 17:08 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-05 11:53 [PATCH v3] qga: fence guest-set-time if hwclock not available Cornelia Huck 2019-12-05 13:05 ` Philippe Mathieu-Daudé 2019-12-05 13:12 ` Cornelia Huck 2019-12-05 13:20 ` Philippe Mathieu-Daudé 2019-12-05 14:21 ` Michal Privoznik 2019-12-06 7:17 ` Markus Armbruster 2019-12-09 18:33 ` Cornelia Huck 2019-12-10 16:38 ` Philippe Mathieu-Daudé 2019-12-05 15:24 ` Laszlo Ersek 2019-12-06 7:15 ` Markus Armbruster 2019-12-06 9:02 ` Laszlo Ersek 2019-12-10 17:07 ` Cornelia Huck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).