On Mon, Feb 09, 2015 at 09:47:47AM -0500, Laine Stump wrote:
> On 02/09/2015 08:19 AM, Ján Tomko wrote:
> > On Mon, Feb 09, 2015 at 10:58:29AM +0100, Michal Privoznik wrote:
> >> Instead of verbose string to enum conversion (if STREQ() else if
> >> STREQ() else if STREQ() ...) lets use awesome VIR_ENUM_* macros.
> >>
> >> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
> >> ---
> >> tools/virsh-domain.c | 28 ++++++++++++++++++----------
> >> 1 file changed, 18 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> >> index b465eb5..c7e4926 100644
> >> --- a/tools/virsh-domain.c
> >> +++ b/tools/virsh-domain.c
> >> @@ -855,10 +855,16 @@ static int parseRateStr(const char *rateStr,
> >> virNetDevBandwidthRatePtr rate)
> >> }
> >>
> >> typedef enum {
> >> - IFACE_TYPE_NETWORK,
> >> + IFACE_TYPE_NETWORK = 0,
> >> IFACE_TYPE_BRIDGE,
> >> + IFACE_TYPE_LAST,
> >> } vshCmdAttachInterfaceType;
> >>
> >> +VIR_ENUM_DECL(vshCmdAttachInterface)
> >> +VIR_ENUM_IMPL(vshCmdAttachInterface, IFACE_TYPE_LAST,
> >> + "network",
> >> + "bridge")
> >> +
> >> static bool
> >> cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
> >> {
> >> @@ -906,11 +912,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
> >> goto cleanup;
> >>
> >> /* check interface type */
> >> - if (STREQ(type, "network")) {
> >> - typ = IFACE_TYPE_NETWORK;
> >> - } else if (STREQ(type, "bridge")) {
> >> - typ = IFACE_TYPE_BRIDGE;
> >> - } else {
> >> + if ((typ = vshCmdAttachInterfaceTypeFromString(type)) < 0) {
> >> vshError(ctl, _("No support for %s in command
> >> 'attach-interface'"),
> >> type);
> >> goto cleanup;
> >> @@ -943,10 +945,16 @@ cmdAttachInterface(vshControl *ctl, const vshCmd
> >> *cmd)
> >> virBufferAsprintf(&buf, "<interface type='%s'>\n", type);
> > Here, the type argument is already assumed to use values from the
> > virDomainNetType enum.
> > I think we can reuse that type here, instead of creating a new enum.
>
> Except that virDomainNetType is not a part of libvirt's public API, and
> so is not available to an application program like virsh.
> We already include conf/*.h files in virsh. Should they be removed to make virsh a cleaner example of a client app? Or left in to avoid code duplication? Jan
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
