Re: [libvirt] [PATCH 2/6] virsh: Create macro for "pool" option
On Wed, Dec 16, 2015 at 08:24:43 -0500, John Ferlan wrote:
[...]
>> static const vshCmdOptDef opts_pool_autostart[] = {
>> - {.name = "pool",
>> - .type = VSH_OT_DATA,
>> - .flags = VSH_OFLAG_REQ,
>> - .help = N_("pool name or uuid")
>> - },
>> + OPT_POOL_COMMON
VSH_POOL_OPT_COMMON for consistency with basically all the other macros.
Also please don't include the trailing comma in the macro, it looks
weird c-wise, and will make possible regex matches for code consistency
really weird.
>> +
>> {.name = "disable",
>> .type = VSH_OT_BOOL,
>> .help = N_("disable autostarting")
>
> Nice. ACK
>
> Should we do something similar to domain, network, ... in the rest of virsh?
>
Hiding stuff behind macros can be confusing sometimes, but for the most
commonly used objects it would be reasonable.
I think it would make certain things easier - I did remark on this in
the cover letter. It gets a bit tedious to do and review; however, I
think in the long run makes things more consistent. There are a few
options with "slight" differences, but nothing that cannot be worked out.
Easier? not really, it would just introduce less code when adding new
stuff at the slight cost of having to resolve the macro when looking up
what the stuff is doing. That's why only the most common objects should
be converted.
Peter
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)