12:12 p.m.
Hi,
This patch fixes the trouble if execute virsh with
virsh --log --debug.
In this case FILE "--debug" is generated and
--debug is off.
This patch treat as error return
if it uses the hyphen "-" at the head of log file name.
Signed-off-by: Atsushi SAKAI <sakaia(a)jp.fujitsu.com>
Thanks
Atsushi SAKAI
12:35 p.m.
Atsushi SAKAI wrote:
Hi,
This patch fixes the trouble if execute virsh with
virsh --log --debug.
In this case FILE "--debug" is generated and
--debug is off.
This patch treat as error return
if it uses the hyphen "-" at the head of log file name.
+1 with reservations.
I'm a bit surprised that getopt_long doesn't do the right thing here.
With your patch we won't be able to have logfiles which begin with a '-'
character. I'm not sure whether that's going to be a problem.
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
12:38 p.m.
On Tue, Jul 31, 2007 at 08:12:56PM +0900, Atsushi SAKAI wrote:
Hi,
This patch fixes the trouble if execute virsh with
virsh --log --debug.
In this case FILE "--debug" is generated and
--debug is off.
Well it does what the user asked, is that really an error ?
Trying to guess what the user meant instead of executing what it asked for
is a good way to break other things. I'm not sure this should be done that
way.
This patch treat as error return
if it uses the hyphen "-" at the head of log file name.
If you really want to catch this then I will ask you to also update the
documentation for that option to explain that using a filename starting with
- is not accepted.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
2:44 p.m.
On Tue, Jul 31, 2007 at 08:12:56PM +0900, Atsushi SAKAI wrote:
Hi,
This patch fixes the trouble if execute virsh with
virsh --log --debug.
In this case FILE "--debug" is generated and
--debug is off.
This patch treat as error return
if it uses the hyphen "-" at the head of log file name.
NACK. The code is already doing exactly what the user asked. The --log
flag takes an argument and you are supplied --debug as that argument.
Therefore it creates the file --debug. This is fine.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
2:46 p.m.
On Tue, Jul 31, 2007 at 07:38:18AM -0400, Daniel Veillard wrote:
On Tue, Jul 31, 2007 at 08:12:56PM +0900, Atsushi SAKAI wrote:
> Hi,
>
> This patch fixes the trouble if execute virsh with
> virsh --log --debug.
> In this case FILE "--debug" is generated and
> --debug is off.
Well it does what the user asked, is that really an error ?
Trying to guess what the user meant instead of executing what it asked for
is a good way to break other things. I'm not sure this should be done that
way.
> This patch treat as error return
> if it uses the hyphen "-" at the head of log file name.
If you really want to catch this then I will ask you to also update the
documentation for that option to explain that using a filename starting with
- is not accepted.
Trying to catch this is a can of worms. You want to put similar 'validation'
into every other command line option ? This is special casing --log and
ignoring the same problem in every other command line option we have. It
is a waste of time & uneccessary complication. Every other UNIX command using
getopt has the same behaviour.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
7:32 a.m.
Hi, Dan
Then This kind of Warning is acceptable?
Thanks
Atsushi SAKAI
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Jul 31, 2007 at 08:12:56PM +0900, Atsushi SAKAI wrote:
> Hi,
>
> This patch fixes the trouble if execute virsh with
> virsh --log --debug.
> In this case FILE "--debug" is generated and
> --debug is off.
>
> This patch treat as error return
> if it uses the hyphen "-" at the head of log file name.
NACK. The code is already doing exactly what the user asked. The --log
flag takes an argument and you are supplied --debug as that argument.
Therefore it creates the file --debug. This is fine.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
1:13 p.m.
Atsushi SAKAI wrote:
Hi, Dan
Then This kind of Warning is acceptable?
I don't have a problem with this sort of warning. Anyone else...?
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
1:21 p.m.
On Mon, Aug 20, 2007 at 01:13:54PM +0100, Richard W.M. Jones wrote:
Atsushi SAKAI wrote:
>Hi, Dan
>
>Then This kind of Warning is acceptable?
I don't have a problem with this sort of warning. Anyone else...?
Hum, why not, though if we start adding warnings, then I would
add a vshWarning function, inspired from vshError, and allow a --nowarning
flag to avoid them. It's okay to be a bit pedantic, as long as the user
has a way around if he knows what he's doing.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
2:28 p.m.
On Mon, Aug 20, 2007 at 08:21:53AM -0400, Daniel Veillard wrote:
On Mon, Aug 20, 2007 at 01:13:54PM +0100, Richard W.M. Jones wrote:
> Atsushi SAKAI wrote:
> >Hi, Dan
> >
> >Then This kind of Warning is acceptable?
>
> I don't have a problem with this sort of warning. Anyone else...?
Hum, why not, though if we start adding warnings, then I would
add a vshWarning function, inspired from vshError, and allow a --nowarning
flag to avoid them. It's okay to be a bit pedantic, as long as the user
has a way around if he knows what he's doing.
Personally I'm against adding a warning for the same reasons as I'm against
making it a hard error. The existing behaviour is standard behaviour of all
UNIX command line tools. If the flag requires an argument no other tool
tries to second guess whether the value supplied is bogus or not, because
it will fundamentally always be a *guess*. Erroring, or printing warnings
about things you're guessing might be wrong is just an annoyance
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
6530
days inactive
6550
days old
8 comments
4 participants
participants (4)
-
Atsushi SAKAI -
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones