On Mon, 21 Mar 2022 10:14:57 +0100 Michal Prívozník <mpriv...@redhat.com> wrote:
> On 3/20/22 22:38, Andrew Deason wrote: > > The code for guest-network-get-interfaces needs a couple of small > > adjustments for Solaris: > > > > - The results from SIOCGIFHWADDR are documented as being in ifr_addr, > > not ifr_hwaddr (ifr_hwaddr doesn't exist on Solaris). > > > > - The implementation of guest_get_network_stats is Linux-specific, so > > hide it under #ifdef CONFIG_LINUX. On non-Linux, we just won't > > provide network interface stats. > > > > Signed-off-by: Andrew Deason <adea...@sinenomine.net> > > --- > > qga/commands-posix.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > > index bd0d67f674..c0b00fc488 100644 > > --- a/qga/commands-posix.c > > +++ b/qga/commands-posix.c > > @@ -2781,20 +2781,21 @@ guest_find_interface(GuestNetworkInterfaceList > > *head, > > return head->value; > > } > > } > > > > return NULL; > > } > > > > static int guest_get_network_stats(const char *name, > > GuestNetworkInterfaceStat *stats) > > { > > +#ifdef CONFIG_LINUX > > int name_len; > > char const *devinfo = "/proc/net/dev"; > > FILE *fp; > > char *line = NULL, *colon; > > size_t n = 0; > > fp = fopen(devinfo, "r"); > > if (!fp) { > > return -1; > > } > > name_len = strlen(name); > > @@ -2836,20 +2837,21 @@ static int guest_get_network_stats(const char *name, > > stats->tx_errs = tx_errs; > > stats->tx_dropped = tx_dropped; > > fclose(fp); > > g_free(line); > > return 0; > > } > > } > > fclose(fp); > > g_free(line); > > g_debug("/proc/net/dev: Interface '%s' not found", name); > > +#endif /* CONFIG_LINUX */ > > I wonder whether we should signal this somehow. I mean, have something > like this: > > #else /* !CONFIG_LINUX */ > g_debug("Stats reporting available only for Linux"); > #endif /* !CONFIG_LINUX */ > > > return -1; > > } > > A counter argument is that if fopen() above fails then -1 is returned > without any error/debug message reported. And stats fetching is best > effort anyway. Ping for this stack. Should I just go ahead and add the above? Sorry if I was expected to respond to this; I don't disagree but I also saw the existing silent-failure code path so it doesn't seem like it matters. I could add debug messages for both silent-failure code paths, maybe as a separate commit afterwards? -- Andrew Deason adea...@sinenomine.net