| Message ID | 20170529091339.1449-1-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/29/2017 05:13 AM, Juergen Gross wrote: > When registering for the Xenstore watch of the node control/sysrq the > handler will be called at once. Don't issue an error message if the > Xenstore node isn't there, as it will be created only when an event > is being triggered. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > drivers/xen/manage.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c > index c1ec8ee80924..7ddd0803da23 100644 > --- a/drivers/xen/manage.c > +++ b/drivers/xen/manage.c > @@ -277,8 +277,11 @@ static void sysrq_handler(struct xenbus_watch *watch, const char *path, > err = xenbus_transaction_start(&xbt); > if (err) > return; > - if (xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key) < 0) { > - pr_err("Unable to read sysrq code in control/sysrq\n"); > + err = xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key); > + if (err < 0) { > + if (err != -ENOENT) Can we distinguish initialization invocation from actual watch firing? E.g. '|| (system_state >= SYSTEM_RUNNING)'? -boris > + pr_err("Error %d reading sysrq code in control/sysrq\n", > + err); > xenbus_transaction_end(xbt, 1); > return; > }
On 30/05/17 15:25, Boris Ostrovsky wrote: > On 05/29/2017 05:13 AM, Juergen Gross wrote: >> When registering for the Xenstore watch of the node control/sysrq the >> handler will be called at once. Don't issue an error message if the >> Xenstore node isn't there, as it will be created only when an event >> is being triggered. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> drivers/xen/manage.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c >> index c1ec8ee80924..7ddd0803da23 100644 >> --- a/drivers/xen/manage.c >> +++ b/drivers/xen/manage.c >> @@ -277,8 +277,11 @@ static void sysrq_handler(struct xenbus_watch *watch, const char *path, >> err = xenbus_transaction_start(&xbt); >> if (err) >> return; >> - if (xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key) < 0) { >> - pr_err("Unable to read sysrq code in control/sysrq\n"); >> + err = xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key); >> + if (err < 0) { >> + if (err != -ENOENT) > > Can we distinguish initialization invocation from actual watch firing? > E.g. '|| (system_state >= SYSTEM_RUNNING)'? The watch will fire again after suspend/resume (e.g. live migration). Juergen > > -boris > >> + pr_err("Error %d reading sysrq code in control/sysrq\n", >> + err); >> xenbus_transaction_end(xbt, 1); >> return; >> } > >
On 05/30/2017 11:03 AM, Juergen Gross wrote: > On 30/05/17 15:25, Boris Ostrovsky wrote: >> On 05/29/2017 05:13 AM, Juergen Gross wrote: >>> When registering for the Xenstore watch of the node control/sysrq the >>> handler will be called at once. Don't issue an error message if the >>> Xenstore node isn't there, as it will be created only when an event >>> is being triggered. >>> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> --- >>> drivers/xen/manage.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c >>> index c1ec8ee80924..7ddd0803da23 100644 >>> --- a/drivers/xen/manage.c >>> +++ b/drivers/xen/manage.c >>> @@ -277,8 +277,11 @@ static void sysrq_handler(struct xenbus_watch *watch, const char *path, >>> err = xenbus_transaction_start(&xbt); >>> if (err) >>> return; >>> - if (xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key) < 0) { >>> - pr_err("Unable to read sysrq code in control/sysrq\n"); >>> + err = xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key); >>> + if (err < 0) { >>> + if (err != -ENOENT) >> Can we distinguish initialization invocation from actual watch firing? >> E.g. '|| (system_state >= SYSTEM_RUNNING)'? > The watch will fire again after suspend/resume (e.g. live migration). That's unfortunate. (And system_state check would also not be a good solution btw since the watch might be processed by the watch thread after we enter SYSTEM_RUNNING). Can you add a comment explaining why we are ignoring ENOENT? -boris > > > Juergen > >> -boris >> >>> + pr_err("Error %d reading sysrq code in control/sysrq\n", >>> + err); >>> xenbus_transaction_end(xbt, 1); >>> return; >>> } >>
On 30/05/17 19:08, Boris Ostrovsky wrote: > On 05/30/2017 11:03 AM, Juergen Gross wrote: >> On 30/05/17 15:25, Boris Ostrovsky wrote: >>> On 05/29/2017 05:13 AM, Juergen Gross wrote: >>>> When registering for the Xenstore watch of the node control/sysrq the >>>> handler will be called at once. Don't issue an error message if the >>>> Xenstore node isn't there, as it will be created only when an event >>>> is being triggered. >>>> >>>> Signed-off-by: Juergen Gross <jgross@suse.com> >>>> --- >>>> drivers/xen/manage.c | 7 +++++-- >>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c >>>> index c1ec8ee80924..7ddd0803da23 100644 >>>> --- a/drivers/xen/manage.c >>>> +++ b/drivers/xen/manage.c >>>> @@ -277,8 +277,11 @@ static void sysrq_handler(struct xenbus_watch *watch, const char *path, >>>> err = xenbus_transaction_start(&xbt); >>>> if (err) >>>> return; >>>> - if (xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key) < 0) { >>>> - pr_err("Unable to read sysrq code in control/sysrq\n"); >>>> + err = xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key); >>>> + if (err < 0) { >>>> + if (err != -ENOENT) >>> Can we distinguish initialization invocation from actual watch firing? >>> E.g. '|| (system_state >= SYSTEM_RUNNING)'? >> The watch will fire again after suspend/resume (e.g. live migration). > > > That's unfortunate. (And system_state check would also not be a good > solution btw since the watch might be processed by the watch thread > after we enter SYSTEM_RUNNING). > > Can you add a comment explaining why we are ignoring ENOENT? Sure. Juergen
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c index c1ec8ee80924..7ddd0803da23 100644 --- a/drivers/xen/manage.c +++ b/drivers/xen/manage.c @@ -277,8 +277,11 @@ static void sysrq_handler(struct xenbus_watch *watch, const char *path, err = xenbus_transaction_start(&xbt); if (err) return; - if (xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key) < 0) { - pr_err("Unable to read sysrq code in control/sysrq\n"); + err = xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key); + if (err < 0) { + if (err != -ENOENT) + pr_err("Error %d reading sysrq code in control/sysrq\n", + err); xenbus_transaction_end(xbt, 1); return; }
When registering for the Xenstore watch of the node control/sysrq the handler will be called at once. Don't issue an error message if the Xenstore node isn't there, as it will be created only when an event is being triggered. Signed-off-by: Juergen Gross <jgross@suse.com> --- drivers/xen/manage.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)