Joao Martins wrote:
> On 09/23/2015 08:18 PM, Jim Fehlig wrote:
>
>> Joao Martins wrote:
>>
>>> Introduce support for domainInterfaceStats API call for querying
>>> network interface statistics. Consequently it also enables the
>>> use of `virsh domifstat <dom> <interface name>` command.
>>>
>>> For getting statistics we resort to virNetInterfaceStats and let
>>> libvirt handle the platform specific nits. Note that the latter
>>> is not yet supported in FreeBSD.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>>> ---
>>> src/libxl/libxl_driver.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 53 insertions(+)
>>>
>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>> index 43e9e47..dc83083 100644
>>> --- a/src/libxl/libxl_driver.c
>>> +++ b/src/libxl/libxl_driver.c
>>> @@ -58,6 +58,7 @@
>>> #include "virhostdev.h"
>>> #include "network/bridge_driver.h"
>>> #include "locking/domain_lock.h"
>>> +#include "virstats.h"
>>>
>>> #define VIR_FROM_THIS VIR_FROM_LIBXL
>>>
>>> @@ -4640,6 +4641,57 @@ libxlDomainIsUpdated(virDomainPtr dom)
>>> }
>>>
>>> static int
>>> +libxlDomainInterfaceStats(virDomainPtr dom,
>>> + const char *path,
>>> + virDomainInterfaceStatsPtr stats)
>>> +{
>>> + libxlDriverPrivatePtr driver = dom->conn->privateData;
>>> + virDomainObjPtr vm;
>>> + int ret = -1;
>>> + int domid, devid;
>>> +
>>> + if (!(vm = libxlDomObjFromDomain(dom)))
>>> + goto cleanup;
>>> +
>>> + if (virDomainInterfaceStatsEnsureACL(dom->conn, vm->def) < 0)
>>> + goto cleanup;
>>> +
>>> + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_QUERY) < 0)
>>> + goto cleanup;
>>> +
>>> + if (!virDomainObjIsActive(vm)) {
>>> + virReportError(VIR_ERR_OPERATION_INVALID,
>>> + "%s", _("domain is not running"));
>>> + goto endjob;
>>> + }
>>> +
>>> + if (sscanf(path, "vif%d.%d", &domid, &devid) != 2) {
>>> + virReportError(VIR_ERR_INVALID_ARG, "%s",
>>> + _("invalid path, unknown device"));
>>> + goto endjob;
>>> + }
>>> +
>>> + if (domid != vm->def->id) {
>>> + virReportError(VIR_ERR_INVALID_ARG, "%s",
>>> + _("invalid path, domid doesn't match"));
>>> + goto endjob;
>>> + }
>>>
>>>
>> Should we also ensure the domain has an interface matching devid before
>> calling virNetInterfaceStats()? I see the qemu driver has such a check,
>> but virNetInterfaceStats() also reports "Interface not found".
>>
>>
> Ultimately I rely on that error and check if the path name is the path
> correspondent to a xen interface prefixed by "vif%d.%d". Problem with doing that
> check is that net->ifname is not set unless explicitly. And looking at the qemu
> driver: they create the device and fetch the ifname and then add it to the bridge.
>
> Perhaps it is reasonable is if we set the net->ifname after domain create and
> then make that check, as you suggest, instead of trying to "guess" the path. The
> way this patch proposes, the user wouldn't be able to get statistics from
> anything other than "vif%d.%d[-emu]" which is incorrect.
>
Yeah, I think setting net->ifname after start is a good idea.
libxlConsoleCallback() could be re-purposed to a more generic
libxlDomainStartCallback() or similar, where info provided by libxl is
copied to the virDomainObj.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list