[libvirt] [PATCH] Add virsh commands for virInterface* functions.

[Daniel P. Berrange]

On Mon, Jun 15, 2009 at 10:56:41PM -0400, Laine Stump wrote:
> >>+    vshPrintExtra(ctl, "%-20s %-30s\n", _("Name"), _("MAC Address"));
> >>+    vshPrintExtra(ctl, 
> >>"--------------------------------------------------\n");
> >>+
> >>+    for (i = 0; i < ifCount; i++) {
> >>+        virInterfacePtr iface = virInterfaceLookupByName(ctl->conn, 
> >>ifNames[i]);
> >>+
> >>+        /* this kind of work with interfaces is not atomic operation */
> >>+        if (!iface) {
> >>+            free(ifNames[i]);
> >>+            continue;
> >>+        }
> >>+
> >>+        vshPrint(ctl, "%-20s %-30s\n",
> >>+                 virInterfaceGetName(iface),
> >>+                 virInterfaceGetMACString(iface));
> >>+        virInterfaceFree(iface);
> >>+        free(ifNames[i]);
> >>+    }
> >>+    free(ifNames);
> >>+    return TRUE;
> >>+}
> >>    
> >
> >
> >This one should also take the flags --inactive / --all to allow
> >selection of offline NICs vs online NICs. Which reminds me that
> >the current public APIs are missing the equivalent for listing
> >inactive interfaces. eg, virConnectNumOfDefinedInterfaces and
> >virConnectListDefinedInterfaces
> >  
> 
> 
> They're missing it because netcf is missing it, and I've so far just 
> been implementing a conduit to channel netcf functionality through 
> libvirt. The problem is that netcf currently interacts with the host OS 
> in only two ways: 1) modifying the ifcfg-XXX config files, and 2) 
> exec'ing the if-up and if-down scripts, and has nothing to get current 
> state of the interface.

If netcf can't do it, it is really very trivial to do in libvirt.
All you need todo is use the SIOCIFFLAGS ioctl() and check for either
the interface not existing at all, or the IFF_UP bit not being set.
We're already doing this in bridge.c for the virNetwork code and
also for QEMU tap devices.

> I talked to lutter about this and he agrees that this would be useful 
> functionality to add to netcf, it just needs a person to do it (I'll 
> even volunteer, just not this week!). Possibly we could add a flag now 
> to the virConnectListInterfaces() API function in libvirt, in 
> anticipation of a similar flag being added to netcf's 
> ncf_list_interfaces() function. Is it too late to modify the API? (it is 
> in a minor release, but nobody has used it yet. I guess the problem 
> would be if someone happened to run libvirt 0.6.4 with an application 
> that *did* use the new functions.). (Now I'm wondering why, when we 
> thought of adding flags to all those other API functions, we didn't 
> think to add flags to this one :-( )

There is no need to modify the API, just follow the model used by all
the other APIs, where we have separate APIs for active vs inactive
objects

There two APIs only list active interfaces

 virConnectListInterfaces() 
 virConnectNumOfInterfaces() 

While these two APIs should list inative interfaces

 virConnectListDefinedInterfaces() 
 virConnectNumOfDefinedInterfaces() 


> 
> Alternately (or maybe additionally) we can/should add a new function to 
> return current state/statistics for interfaces. Along with 
> throughput/packet counts, that API could return flags for the interface 
> (all the flags shown in the output of ifconfig) as well as the current 
> IP address(es). The virsh command could then get the list of interfaces, 
> and iterate through looking at the state of each, eliminating the 
> interfaces that weren't UP (or DOWN).
> 
> >
> >
> >This command should be called 'start' for consistency with other commands.
> >
> >'create' is the command name we use for creating a transient object,
> >rather than starting a persistent object.
> >  
> 
> I see what you mean - "net-start" hooks up to virNetworkCreate(), so 
> virInterfaceCreate() should be connected to "iface-start". It's a bit 
> confusing, though, that the virsh commands don't match the libvirt API 
> names. (I guess at least they should *consistently* mismatch! ;-)

Consistency is my number 1 priority with API & virsh command naming, such
that once users know the libvirt naming scheme, it can be predictably 
followed for all APIs.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|