[libvirt] [PATCH v3 0/5] virsh: Further improve handling of integer options

Andrea Bolognani abologna at redhat.com
Tue Jun 2 09:35:56 UTC 2015


On Sun, 2015-05-31 at 15:19 -0400, John Ferlan wrote:

First of all, thanks for the review :)

> Patch 2 had some comments which should be simple to fix

I've commented in detail in your other mail, should have taken care of
them all.

> Patch 4 had a couple of NIT's about going beyond 80 columns on a line,
> but since it's so pervasive in the rest of the module that can be a
> future cleanup ;-)...

Sure, there's always time for cleaning up after the cleanup :)

> I do note that 'vshCommandOpt' now has an
> unrelated difference - I assume it had a ctl argument at one point
> that's since been removed...

I'm not sure what you mean here, it looks fine to me. Care to explain in
more detail?

> Patch 5 still seemed to need some sort of error message in
> vshCommandOptString... Some callers ignore return status, so even adding
> an error may still be "ignored".

This series deals with numeric options only. String options are
something I will probably tackle in the near future :)

> Overall seems OK to me with some minor cleanups.

Looks like this series is growing a new commit for every subsequent
review! I've just posted v4, which should address your comments. Please
take a look at it and let me know if there's more work to do.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team
$ python -c "print('a'.join(['', 'bologn', '@redh', 't.com']))"




More information about the libvir-list mailing list