* [PATCH] tools/xenstore-watch: Add new timeout parameter
@ 2016-03-16 15:50 Razvan Cojocaru
2016-03-16 17:43 ` Konrad Rzeszutek Wilk
2016-03-16 18:46 ` Wei Liu
0 siblings, 2 replies; 7+ messages in thread
From: Razvan Cojocaru @ 2016-03-16 15:50 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, ian.jackson, Razvan Cojocaru, stefano.stabellini
This patch allows xenstore-watch to exit even if no changes to its
XenStore key have occured in a specified interval (in seconds), via
a new -T parameter.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
tools/xenstore/xenstore_client.c | 64 ++++++++++++++++++++++++++++++----------
1 file changed, 48 insertions(+), 16 deletions(-)
diff --git a/tools/xenstore/xenstore_client.c b/tools/xenstore/xenstore_client.c
index 3d14d37..2873b71 100644
--- a/tools/xenstore/xenstore_client.c
+++ b/tools/xenstore/xenstore_client.c
@@ -12,6 +12,7 @@
#include <errno.h>
#include <fcntl.h>
#include <getopt.h>
+#include <poll.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
@@ -99,7 +100,7 @@ usage(enum mode mode, int incl_mode, const char *progname)
errx(1, "Usage: %s %s[-h] [-u] [-r] [-s] key <mode [modes...]>", progname, mstr);
case MODE_watch:
mstr = incl_mode ? "watch " : "";
- errx(1, "Usage: %s %s[-h] [-n NR] key", progname, mstr);
+ errx(1, "Usage: %s %s[-h] [-n NR] [-T TIMEOUT] key", progname, mstr);
}
}
@@ -273,27 +274,49 @@ do_chmod(char *path, struct xs_permissions *perms, int nperms, int upto,
}
static void
-do_watch(struct xs_handle *xsh, int max_events)
+do_watch(struct xs_handle *xsh, int max_events, int timeout)
{
- int count = 0;
+ int rc, ms_timeout = timeout * 1000;
char **vec = NULL;
+ struct pollfd fd;
- for ( count = 0; max_events == -1 || count < max_events; count++ ) {
- unsigned int num;
+ fd.fd = xs_fileno(xsh);
+ fd.events = POLLIN | POLLERR;
- vec = xs_read_watch(xsh, &num);
- if (vec == NULL)
- continue;
+ do {
+ rc = poll(&fd, 1, 100);
- printf("%s\n", vec[XS_WATCH_PATH]);
- fflush(stdout);
- free(vec);
- }
+ if (rc == 0 || (rc < 0 && errno == EINTR)) {
+ ms_timeout -= 100;
+ continue;
+ }
+
+ if (rc < 0) {
+ err(1, "poll() failed: %s", strerror(errno));
+ return;
+ }
+
+ if (fd.revents & POLLIN) {
+ unsigned int num;
+
+ --max_events;
+ vec = xs_read_watch(xsh, &num);
+
+ if (vec == NULL)
+ continue;
+
+ printf("%s\n", vec[XS_WATCH_PATH]);
+ fflush(stdout);
+ free(vec);
+ }
+
+ } while ((ms_timeout > 0 || timeout == -1) && max_events > 0);
}
static int
perform(enum mode mode, int optind, int argc, char **argv, struct xs_handle *xsh,
- xs_transaction_t xth, int prefix, int tidy, int upto, int recurse, int nr_watches)
+ xs_transaction_t xth, int prefix, int tidy, int upto, int recurse, int nr_watches,
+ int timeout)
{
switch (mode) {
case MODE_ls:
@@ -461,7 +484,7 @@ perform(enum mode mode, int optind, int argc, char **argv, struct xs_handle *xsh
if (!xs_watch(xsh, w, w))
errx(1, "Unable to add watch on %s\n", w);
}
- do_watch(xsh, nr_watches);
+ do_watch(xsh, nr_watches, timeout);
}
}
}
@@ -505,6 +528,7 @@ main(int argc, char **argv)
int upto = 0;
int recurse = 0;
int nr_watches = -1;
+ int timeout = -1;
int transaction;
struct winsize ws;
enum mode mode;
@@ -539,10 +563,11 @@ main(int argc, char **argv)
{"upto", 0, 0, 'u'}, /* MODE_chmod */
{"recurse", 0, 0, 'r'}, /* MODE_chmod */
{"number", 1, 0, 'n'}, /* MODE_watch */
+ {"timeout", 1, 0, 'T'}, /* MODE_watch */
{0, 0, 0, 0}
};
- c = getopt_long(argc - switch_argv, argv + switch_argv, "hfspturn:",
+ c = getopt_long(argc - switch_argv, argv + switch_argv, "hfspturn:T:",
long_options, &index);
if (c == -1)
break;
@@ -593,6 +618,12 @@ main(int argc, char **argv)
else
usage(mode, switch_argv, argv[0]);
break;
+ case 'T':
+ if ( mode == MODE_watch )
+ timeout = atoi(optarg);
+ else
+ usage(mode, switch_argv, argv[0]);
+ break;
}
}
@@ -646,7 +677,8 @@ again:
errx(1, "couldn't start transaction");
}
- ret = perform(mode, optind, argc - switch_argv, argv + switch_argv, xsh, xth, prefix, tidy, upto, recurse, nr_watches);
+ ret = perform(mode, optind, argc - switch_argv, argv + switch_argv, xsh, xth, prefix, tidy, upto, recurse,
+ nr_watches, timeout);
if (transaction && !xs_transaction_end(xsh, xth, ret)) {
if (ret == 0 && errno == EAGAIN) {
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/xenstore-watch: Add new timeout parameter
2016-03-16 15:50 [PATCH] tools/xenstore-watch: Add new timeout parameter Razvan Cojocaru
@ 2016-03-16 17:43 ` Konrad Rzeszutek Wilk
2016-03-16 17:50 ` Andrew Cooper
2016-03-16 18:46 ` Wei Liu
1 sibling, 1 reply; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-16 17:43 UTC (permalink / raw)
To: Razvan Cojocaru; +Cc: ian.jackson, stefano.stabellini, wei.liu2, xen-devel
On Wed, Mar 16, 2016 at 05:50:46PM +0200, Razvan Cojocaru wrote:
> This patch allows xenstore-watch to exit even if no changes to its
> XenStore key have occured in a specified interval (in seconds), via
> a new -T parameter.
Should there be an update to the manpage as well?
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> ---
> tools/xenstore/xenstore_client.c | 64 ++++++++++++++++++++++++++++++----------
> 1 file changed, 48 insertions(+), 16 deletions(-)
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/xenstore-watch: Add new timeout parameter
2016-03-16 17:43 ` Konrad Rzeszutek Wilk
@ 2016-03-16 17:50 ` Andrew Cooper
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2016-03-16 17:50 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Razvan Cojocaru
Cc: wei.liu2, xen-devel, ian.jackson, stefano.stabellini
On 16/03/16 17:43, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 16, 2016 at 05:50:46PM +0200, Razvan Cojocaru wrote:
>> This patch allows xenstore-watch to exit even if no changes to its
>> XenStore key have occured in a specified interval (in seconds), via
>> a new -T parameter.
> Should there be an update to the manpage as well?
There are currently isn't a manpage covering xenstore-watch, and the
more general xenstore(1) refers to the non-existent xenstore-watch(1).
Introducing one would be great. xenstore-ls looks like a good example.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/xenstore-watch: Add new timeout parameter
2016-03-16 15:50 [PATCH] tools/xenstore-watch: Add new timeout parameter Razvan Cojocaru
2016-03-16 17:43 ` Konrad Rzeszutek Wilk
@ 2016-03-16 18:46 ` Wei Liu
2016-03-16 18:52 ` Razvan Cojocaru
1 sibling, 1 reply; 7+ messages in thread
From: Wei Liu @ 2016-03-16 18:46 UTC (permalink / raw)
To: Razvan Cojocaru; +Cc: ian.jackson, stefano.stabellini, wei.liu2, xen-devel
On Wed, Mar 16, 2016 at 05:50:46PM +0200, Razvan Cojocaru wrote:
[...]
> }
>
> @@ -273,27 +274,49 @@ do_chmod(char *path, struct xs_permissions *perms, int nperms, int upto,
> }
>
> static void
> -do_watch(struct xs_handle *xsh, int max_events)
> +do_watch(struct xs_handle *xsh, int max_events, int timeout)
> {
> - int count = 0;
> + int rc, ms_timeout = timeout * 1000;
> char **vec = NULL;
> + struct pollfd fd;
>
> - for ( count = 0; max_events == -1 || count < max_events; count++ ) {
> - unsigned int num;
> + fd.fd = xs_fileno(xsh);
> + fd.events = POLLIN | POLLERR;
>
> - vec = xs_read_watch(xsh, &num);
> - if (vec == NULL)
> - continue;
> + do {
> + rc = poll(&fd, 1, 100);
>
> - printf("%s\n", vec[XS_WATCH_PATH]);
> - fflush(stdout);
> - free(vec);
> - }
> + if (rc == 0 || (rc < 0 && errno == EINTR)) {
> + ms_timeout -= 100;
Shouldn't you just exit the loop when getting EINTR?
And as others have mentioned, please also patch manpage.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/xenstore-watch: Add new timeout parameter
2016-03-16 18:46 ` Wei Liu
@ 2016-03-16 18:52 ` Razvan Cojocaru
2016-03-16 18:57 ` Wei Liu
0 siblings, 1 reply; 7+ messages in thread
From: Razvan Cojocaru @ 2016-03-16 18:52 UTC (permalink / raw)
To: Wei Liu; +Cc: stefano.stabellini, ian.jackson, xen-devel
On 03/16/2016 08:46 PM, Wei Liu wrote:
> On Wed, Mar 16, 2016 at 05:50:46PM +0200, Razvan Cojocaru wrote:
> [...]
>> }
>>
>> @@ -273,27 +274,49 @@ do_chmod(char *path, struct xs_permissions *perms, int nperms, int upto,
>> }
>>
>> static void
>> -do_watch(struct xs_handle *xsh, int max_events)
>> +do_watch(struct xs_handle *xsh, int max_events, int timeout)
>> {
>> - int count = 0;
>> + int rc, ms_timeout = timeout * 1000;
>> char **vec = NULL;
>> + struct pollfd fd;
>>
>> - for ( count = 0; max_events == -1 || count < max_events; count++ ) {
>> - unsigned int num;
>> + fd.fd = xs_fileno(xsh);
>> + fd.events = POLLIN | POLLERR;
>>
>> - vec = xs_read_watch(xsh, &num);
>> - if (vec == NULL)
>> - continue;
>> + do {
>> + rc = poll(&fd, 1, 100);
>>
>> - printf("%s\n", vec[XS_WATCH_PATH]);
>> - fflush(stdout);
>> - free(vec);
>> - }
>> + if (rc == 0 || (rc < 0 && errno == EINTR)) {
>> + ms_timeout -= 100;
>
> Shouldn't you just exit the loop when getting EINTR?
I don't think so, an EINTR means that a signal has been caught during
poll() - it's not really a failure case.
> And as others have mentioned, please also patch manpage.
As Andrew has pointed out, there is no man page to patch. I agree that
adding one would be great, but I hope this patch can get through without
it being required - it can be added later.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/xenstore-watch: Add new timeout parameter
2016-03-16 18:52 ` Razvan Cojocaru
@ 2016-03-16 18:57 ` Wei Liu
2016-03-16 19:06 ` Razvan Cojocaru
0 siblings, 1 reply; 7+ messages in thread
From: Wei Liu @ 2016-03-16 18:57 UTC (permalink / raw)
To: Razvan Cojocaru; +Cc: ian.jackson, xen-devel, Wei Liu, stefano.stabellini
On Wed, Mar 16, 2016 at 08:52:57PM +0200, Razvan Cojocaru wrote:
> On 03/16/2016 08:46 PM, Wei Liu wrote:
> > On Wed, Mar 16, 2016 at 05:50:46PM +0200, Razvan Cojocaru wrote:
> > [...]
> >> }
> >>
> >> @@ -273,27 +274,49 @@ do_chmod(char *path, struct xs_permissions *perms, int nperms, int upto,
> >> }
> >>
> >> static void
> >> -do_watch(struct xs_handle *xsh, int max_events)
> >> +do_watch(struct xs_handle *xsh, int max_events, int timeout)
> >> {
> >> - int count = 0;
> >> + int rc, ms_timeout = timeout * 1000;
> >> char **vec = NULL;
> >> + struct pollfd fd;
> >>
> >> - for ( count = 0; max_events == -1 || count < max_events; count++ ) {
> >> - unsigned int num;
> >> + fd.fd = xs_fileno(xsh);
> >> + fd.events = POLLIN | POLLERR;
> >>
> >> - vec = xs_read_watch(xsh, &num);
> >> - if (vec == NULL)
> >> - continue;
> >> + do {
> >> + rc = poll(&fd, 1, 100);
> >>
> >> - printf("%s\n", vec[XS_WATCH_PATH]);
> >> - fflush(stdout);
> >> - free(vec);
> >> - }
> >> + if (rc == 0 || (rc < 0 && errno == EINTR)) {
> >> + ms_timeout -= 100;
> >
> > Shouldn't you just exit the loop when getting EINTR?
>
> I don't think so, an EINTR means that a signal has been caught during
> poll() - it's not really a failure case.
>
What about Ctrl-C ? I think it is reasonable that user want to interrupt
the process.
> > And as others have mentioned, please also patch manpage.
>
> As Andrew has pointed out, there is no man page to patch. I agree that
> adding one would be great, but I hope this patch can get through without
> it being required - it can be added later.
>
Yes, follow-up patch is OK.
Wei.
>
> Thanks,
> Razvan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/xenstore-watch: Add new timeout parameter
2016-03-16 18:57 ` Wei Liu
@ 2016-03-16 19:06 ` Razvan Cojocaru
0 siblings, 0 replies; 7+ messages in thread
From: Razvan Cojocaru @ 2016-03-16 19:06 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, ian.jackson, stefano.stabellini
On 03/16/2016 08:57 PM, Wei Liu wrote:
> On Wed, Mar 16, 2016 at 08:52:57PM +0200, Razvan Cojocaru wrote:
>> On 03/16/2016 08:46 PM, Wei Liu wrote:
>>> On Wed, Mar 16, 2016 at 05:50:46PM +0200, Razvan Cojocaru wrote:
>>> [...]
>>>> }
>>>>
>>>> @@ -273,27 +274,49 @@ do_chmod(char *path, struct xs_permissions *perms, int nperms, int upto,
>>>> }
>>>>
>>>> static void
>>>> -do_watch(struct xs_handle *xsh, int max_events)
>>>> +do_watch(struct xs_handle *xsh, int max_events, int timeout)
>>>> {
>>>> - int count = 0;
>>>> + int rc, ms_timeout = timeout * 1000;
>>>> char **vec = NULL;
>>>> + struct pollfd fd;
>>>>
>>>> - for ( count = 0; max_events == -1 || count < max_events; count++ ) {
>>>> - unsigned int num;
>>>> + fd.fd = xs_fileno(xsh);
>>>> + fd.events = POLLIN | POLLERR;
>>>>
>>>> - vec = xs_read_watch(xsh, &num);
>>>> - if (vec == NULL)
>>>> - continue;
>>>> + do {
>>>> + rc = poll(&fd, 1, 100);
>>>>
>>>> - printf("%s\n", vec[XS_WATCH_PATH]);
>>>> - fflush(stdout);
>>>> - free(vec);
>>>> - }
>>>> + if (rc == 0 || (rc < 0 && errno == EINTR)) {
>>>> + ms_timeout -= 100;
>>>
>>> Shouldn't you just exit the loop when getting EINTR?
>>
>> I don't think so, an EINTR means that a signal has been caught during
>> poll() - it's not really a failure case.
>>
>
> What about Ctrl-C ? I think it is reasonable that user want to interrupt
> the process.
Fair enough, I'll re-spin V2.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-03-16 19:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-16 15:50 [PATCH] tools/xenstore-watch: Add new timeout parameter Razvan Cojocaru
2016-03-16 17:43 ` Konrad Rzeszutek Wilk
2016-03-16 17:50 ` Andrew Cooper
2016-03-16 18:46 ` Wei Liu
2016-03-16 18:52 ` Razvan Cojocaru
2016-03-16 18:57 ` Wei Liu
2016-03-16 19:06 ` Razvan Cojocaru
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).