* [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun)
@ 2014-12-13 16:54 Wei Liu
2014-12-13 18:37 ` Konrad Rzeszutek Wilk
2014-12-15 10:47 ` Ian Campbell
0 siblings, 2 replies; 6+ messages in thread
From: Wei Liu @ 2014-12-13 16:54 UTC (permalink / raw)
To: xen-devel; +Cc: tlviewer, Wei Liu, Ian Campbell, M A Young
In commit d36a3734a ("xl: fix migration failure with xl migrate
--debug"), message is printed to stderr for both debug mode
and dryrun mode. That caused rdname() in xendomains fails to parse
domain name since it's expecting input from xl's stdout.
So this patch separates those two cases. If xl is running in debug mode,
then message is printed to stderr; if xl is running in dryrun mode and
debug is not enabled, message is printed to stdout. This will fix
xendomains and other scripts that use "xl create --dryrun", as well as
not re-introducing the old bug fixed in d36a3734a.
Reported-by: Mark Pryor <tlviewer@yahoo.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: M A Young <m.a.young@durham.ac.uk>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>
---
This is a regression, and this bug is so subtle that's a bit hard to
debug from user's point of view. So I think this should go into 4.5.
Mark posted a workaround in
<104017455.78913.1418434454763.JavaMail.yahoo@jws10624.mail.bf1.yahoo.com>
but to be honest I don't think it's nice to have everyone patch their
scripts in order to work around this.
---
tools/libxl/xl_cmdimpl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3737c7e..0a5f7c8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2472,8 +2472,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
}
}
- if (debug || dom_info->dryrun)
+ if (debug)
printf_info(default_output_format, -1, &d_config, stderr);
+ if (!debug && dom_info->dryrun)
+ printf_info(default_output_format, -1, &d_config, stdout);
ret = 0;
if (dom_info->dryrun)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun)
2014-12-13 16:54 [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun) Wei Liu
@ 2014-12-13 18:37 ` Konrad Rzeszutek Wilk
2014-12-13 18:42 ` Wei Liu
2014-12-15 10:47 ` Ian Campbell
1 sibling, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-12-13 18:37 UTC (permalink / raw)
To: Wei Liu; +Cc: tlviewer, M A Young, Ian Campbell, xen-devel
On Sat, Dec 13, 2014 at 04:54:05PM +0000, Wei Liu wrote:
> In commit d36a3734a ("xl: fix migration failure with xl migrate
> --debug"), message is printed to stderr for both debug mode
> and dryrun mode. That caused rdname() in xendomains fails to parse
> domain name since it's expecting input from xl's stdout.
>
> So this patch separates those two cases. If xl is running in debug mode,
> then message is printed to stderr; if xl is running in dryrun mode and
> debug is not enabled, message is printed to stdout. This will fix
> xendomains and other scripts that use "xl create --dryrun", as well as
> not re-introducing the old bug fixed in d36a3734a.
>
> Reported-by: Mark Pryor <tlviewer@yahoo.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: M A Young <m.a.young@durham.ac.uk>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Konrad Wilk <konrad.wilk@oracle.com>
> ---
> This is a regression, and this bug is so subtle that's a bit hard to
> debug from user's point of view. So I think this should go into 4.5.
>
> Mark posted a workaround in
> <104017455.78913.1418434454763.JavaMail.yahoo@jws10624.mail.bf1.yahoo.com>
> but to be honest I don't think it's nice to have everyone patch their
> scripts in order to work around this.
> ---
> tools/libxl/xl_cmdimpl.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 3737c7e..0a5f7c8 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2472,8 +2472,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
> }
> }
>
> - if (debug || dom_info->dryrun)
> + if (debug)
> printf_info(default_output_format, -1, &d_config, stderr);
> + if (!debug && dom_info->dryrun)
Could this 'else if (dom_info->dry_run)' ?
I know that semantically it is exactly the same as the 'if (!debug ..)' but
it just "feels" more proper? Thought since you are the maintainer in that
area - your opinion on how you want to do that is of course authoritive.
Regardless,
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> + printf_info(default_output_format, -1, &d_config, stdout);
>
> ret = 0;
> if (dom_info->dryrun)
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun)
2014-12-13 18:37 ` Konrad Rzeszutek Wilk
@ 2014-12-13 18:42 ` Wei Liu
2014-12-13 18:46 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2014-12-13 18:42 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: tlviewer, M A Young, Wei Liu, Ian Campbell, xen-devel
On Sat, Dec 13, 2014 at 01:37:16PM -0500, Konrad Rzeszutek Wilk wrote:
> On Sat, Dec 13, 2014 at 04:54:05PM +0000, Wei Liu wrote:
> > In commit d36a3734a ("xl: fix migration failure with xl migrate
> > --debug"), message is printed to stderr for both debug mode
> > and dryrun mode. That caused rdname() in xendomains fails to parse
> > domain name since it's expecting input from xl's stdout.
> >
> > So this patch separates those two cases. If xl is running in debug mode,
> > then message is printed to stderr; if xl is running in dryrun mode and
> > debug is not enabled, message is printed to stdout. This will fix
> > xendomains and other scripts that use "xl create --dryrun", as well as
> > not re-introducing the old bug fixed in d36a3734a.
> >
> > Reported-by: Mark Pryor <tlviewer@yahoo.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: M A Young <m.a.young@durham.ac.uk>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Konrad Wilk <konrad.wilk@oracle.com>
> > ---
> > This is a regression, and this bug is so subtle that's a bit hard to
> > debug from user's point of view. So I think this should go into 4.5.
> >
> > Mark posted a workaround in
> > <104017455.78913.1418434454763.JavaMail.yahoo@jws10624.mail.bf1.yahoo.com>
> > but to be honest I don't think it's nice to have everyone patch their
> > scripts in order to work around this.
> > ---
> > tools/libxl/xl_cmdimpl.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 3737c7e..0a5f7c8 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -2472,8 +2472,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
> > }
> > }
> >
> > - if (debug || dom_info->dryrun)
> > + if (debug)
> > printf_info(default_output_format, -1, &d_config, stderr);
> > + if (!debug && dom_info->dryrun)
>
> Could this 'else if (dom_info->dry_run)' ?
>
> I know that semantically it is exactly the same as the 'if (!debug ..)' but
> it just "feels" more proper? Thought since you are the maintainer in that
> area - your opinion on how you want to do that is of course authoritive.
>
I don't have strong preference on this. I just happened to type in
whatever style that flowed though my mind at that particular point. If
Ian and you both feel strongly about this I can certainly resend. :-)
Wei.
> Regardless,
>
> Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > + printf_info(default_output_format, -1, &d_config, stdout);
> >
> > ret = 0;
> > if (dom_info->dryrun)
> > --
> > 1.7.10.4
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun)
2014-12-13 18:42 ` Wei Liu
@ 2014-12-13 18:46 ` Konrad Rzeszutek Wilk
2014-12-15 10:48 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-12-13 18:46 UTC (permalink / raw)
To: Wei Liu; +Cc: tlviewer, M A Young, Ian Campbell, xen-devel
On Sat, Dec 13, 2014 at 06:42:09PM +0000, Wei Liu wrote:
> On Sat, Dec 13, 2014 at 01:37:16PM -0500, Konrad Rzeszutek Wilk wrote:
> > On Sat, Dec 13, 2014 at 04:54:05PM +0000, Wei Liu wrote:
> > > In commit d36a3734a ("xl: fix migration failure with xl migrate
> > > --debug"), message is printed to stderr for both debug mode
> > > and dryrun mode. That caused rdname() in xendomains fails to parse
> > > domain name since it's expecting input from xl's stdout.
> > >
> > > So this patch separates those two cases. If xl is running in debug mode,
> > > then message is printed to stderr; if xl is running in dryrun mode and
> > > debug is not enabled, message is printed to stdout. This will fix
> > > xendomains and other scripts that use "xl create --dryrun", as well as
> > > not re-introducing the old bug fixed in d36a3734a.
> > >
> > > Reported-by: Mark Pryor <tlviewer@yahoo.com>
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > Cc: M A Young <m.a.young@durham.ac.uk>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Konrad Wilk <konrad.wilk@oracle.com>
> > > ---
> > > This is a regression, and this bug is so subtle that's a bit hard to
> > > debug from user's point of view. So I think this should go into 4.5.
> > >
> > > Mark posted a workaround in
> > > <104017455.78913.1418434454763.JavaMail.yahoo@jws10624.mail.bf1.yahoo.com>
> > > but to be honest I don't think it's nice to have everyone patch their
> > > scripts in order to work around this.
> > > ---
> > > tools/libxl/xl_cmdimpl.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > index 3737c7e..0a5f7c8 100644
> > > --- a/tools/libxl/xl_cmdimpl.c
> > > +++ b/tools/libxl/xl_cmdimpl.c
> > > @@ -2472,8 +2472,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
> > > }
> > > }
> > >
> > > - if (debug || dom_info->dryrun)
> > > + if (debug)
> > > printf_info(default_output_format, -1, &d_config, stderr);
> > > + if (!debug && dom_info->dryrun)
> >
> > Could this 'else if (dom_info->dry_run)' ?
> >
> > I know that semantically it is exactly the same as the 'if (!debug ..)' but
> > it just "feels" more proper? Thought since you are the maintainer in that
> > area - your opinion on how you want to do that is of course authoritive.
> >
>
> I don't have strong preference on this. I just happened to type in
> whatever style that flowed though my mind at that particular point. If
> Ian and you both feel strongly about this I can certainly resend. :-)
Lets wait to see what Ian says (or what he ends up checking in).
>
> Wei.
>
> > Regardless,
> >
> > Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > + printf_info(default_output_format, -1, &d_config, stdout);
> > >
> > > ret = 0;
> > > if (dom_info->dryrun)
> > > --
> > > 1.7.10.4
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun)
2014-12-13 16:54 [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun) Wei Liu
2014-12-13 18:37 ` Konrad Rzeszutek Wilk
@ 2014-12-15 10:47 ` Ian Campbell
1 sibling, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2014-12-15 10:47 UTC (permalink / raw)
To: Wei Liu; +Cc: tlviewer, M A Young, xen-devel
On Sat, 2014-12-13 at 16:54 +0000, Wei Liu wrote:
> In commit d36a3734a ("xl: fix migration failure with xl migrate
> --debug"), message is printed to stderr for both debug mode
> and dryrun mode. That caused rdname() in xendomains fails to parse
> domain name since it's expecting input from xl's stdout.
>
> So this patch separates those two cases. If xl is running in debug mode,
> then message is printed to stderr; if xl is running in dryrun mode and
> debug is not enabled, message is printed to stdout. This will fix
> xendomains and other scripts that use "xl create --dryrun", as well as
> not re-introducing the old bug fixed in d36a3734a.
>
> Reported-by: Mark Pryor <tlviewer@yahoo.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: M A Young <m.a.young@durham.ac.uk>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Konrad Wilk <konrad.wilk@oracle.com>
> ---
> This is a regression, and this bug is so subtle that's a bit hard to
> debug from user's point of view. So I think this should go into 4.5.
Agreed.
> Mark posted a workaround in
> <104017455.78913.1418434454763.JavaMail.yahoo@jws10624.mail.bf1.yahoo.com>
> but to be honest I don't think it's nice to have everyone patch their
> scripts in order to work around this.
Right.
> ---
> tools/libxl/xl_cmdimpl.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 3737c7e..0a5f7c8 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2472,8 +2472,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
> }
> }
>
> - if (debug || dom_info->dryrun)
> + if (debug)
> printf_info(default_output_format, -1, &d_config, stderr);
> + if (!debug && dom_info->dryrun)
else if ( dom_info->dry-run )
is the same (right?) and less thinking for the reader.
Or the whole thing could be just:
if (debug || dom_info->dryrun)
printf_info(default_output_format, -1, &d_config, debug ? stderr : stdout);
Less repetitive.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun)
2014-12-13 18:46 ` Konrad Rzeszutek Wilk
@ 2014-12-15 10:48 ` Ian Campbell
0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2014-12-15 10:48 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: tlviewer, M A Young, Wei Liu, xen-devel
On Sat, 2014-12-13 at 13:46 -0500, Konrad Rzeszutek Wilk wrote:
> On Sat, Dec 13, 2014 at 06:42:09PM +0000, Wei Liu wrote:
> > On Sat, Dec 13, 2014 at 01:37:16PM -0500, Konrad Rzeszutek Wilk wrote:
> > > On Sat, Dec 13, 2014 at 04:54:05PM +0000, Wei Liu wrote:
> > > > In commit d36a3734a ("xl: fix migration failure with xl migrate
> > > > --debug"), message is printed to stderr for both debug mode
> > > > and dryrun mode. That caused rdname() in xendomains fails to parse
> > > > domain name since it's expecting input from xl's stdout.
> > > >
> > > > So this patch separates those two cases. If xl is running in debug mode,
> > > > then message is printed to stderr; if xl is running in dryrun mode and
> > > > debug is not enabled, message is printed to stdout. This will fix
> > > > xendomains and other scripts that use "xl create --dryrun", as well as
> > > > not re-introducing the old bug fixed in d36a3734a.
> > > >
> > > > Reported-by: Mark Pryor <tlviewer@yahoo.com>
> > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > Cc: M A Young <m.a.young@durham.ac.uk>
> > > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > > Cc: Konrad Wilk <konrad.wilk@oracle.com>
> > > > ---
> > > > This is a regression, and this bug is so subtle that's a bit hard to
> > > > debug from user's point of view. So I think this should go into 4.5.
> > > >
> > > > Mark posted a workaround in
> > > > <104017455.78913.1418434454763.JavaMail.yahoo@jws10624.mail.bf1.yahoo.com>
> > > > but to be honest I don't think it's nice to have everyone patch their
> > > > scripts in order to work around this.
> > > > ---
> > > > tools/libxl/xl_cmdimpl.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > > index 3737c7e..0a5f7c8 100644
> > > > --- a/tools/libxl/xl_cmdimpl.c
> > > > +++ b/tools/libxl/xl_cmdimpl.c
> > > > @@ -2472,8 +2472,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
> > > > }
> > > > }
> > > >
> > > > - if (debug || dom_info->dryrun)
> > > > + if (debug)
> > > > printf_info(default_output_format, -1, &d_config, stderr);
> > > > + if (!debug && dom_info->dryrun)
> > >
> > > Could this 'else if (dom_info->dry_run)' ?
> > >
> > > I know that semantically it is exactly the same as the 'if (!debug ..)' but
> > > it just "feels" more proper? Thought since you are the maintainer in that
> > > area - your opinion on how you want to do that is of course authoritive.
> > >
> >
> > I don't have strong preference on this. I just happened to type in
> > whatever style that flowed though my mind at that particular point. If
> > Ian and you both feel strongly about this I can certainly resend. :-)
>
> Lets wait to see what Ian says (or what he ends up checking in).
I really ought to read the thread before replying. I do prefer the else
if style, or the ternary operator one I also proposed.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-15 10:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-13 16:54 [PATCH for-4.5] xl: print message to stdout when (!debug && dryrun) Wei Liu
2014-12-13 18:37 ` Konrad Rzeszutek Wilk
2014-12-13 18:42 ` Wei Liu
2014-12-13 18:46 ` Konrad Rzeszutek Wilk
2014-12-15 10:48 ` Ian Campbell
2014-12-15 10:47 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.