| Message ID | 20221003093911.1844031-7-alexander.ivanov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | qga: Add FreeBSD support | expand |
Hi On Mon, Oct 3, 2022 at 1:39 PM Alexander Ivanov < alexander.ivanov@virtuozzo.com> wrote: > In the next patch FreeBSD support for guest-network-get-interfaces will be > added. Previously move Linux-specific code of HW address getting to a > separate functions and add a dumb function to commands-bsd.c. > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > --- > qga/commands-bsd.c | 16 +++++++ > qga/commands-common.h | 6 +++ > qga/commands-posix.c | 100 ++++++++++++++++++++++++------------------ > 3 files changed, 80 insertions(+), 42 deletions(-) > > diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c > index ca06692179..40f7ec7600 100644 > --- a/qga/commands-bsd.c > +++ b/qga/commands-bsd.c > @@ -167,3 +167,19 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error > **errp) > return NULL; > } > #endif /* CONFIG_FSFREEZE */ > + > +#ifdef HAVE_GETIFADDRS > +/* > + * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a > + * buffer with ETHER_ADDR_LEN length at least. > + * > + * Returns -1 in case of an error, otherwise 0. "obtained" arguument is > + * true if a MAC address was obtained successful, otherwise false. > + */ > +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, > + bool *obtained, Error **errp) > +{ > + *obtained = false; > + return 0; > +} > +#endif /* HAVE_GETIFADDRS */ > diff --git a/qga/commands-common.h b/qga/commands-common.h > index 2d9878a634..a9cdaf7278 100644 > --- a/qga/commands-common.h > +++ b/qga/commands-common.h > @@ -56,6 +56,12 @@ int64_t qmp_guest_fsfreeze_do_freeze_list(bool > has_mountpoints, > int qmp_guest_fsfreeze_do_thaw(Error **errp); > #endif /* CONFIG_FSFREEZE */ > > +#ifdef HAVE_GETIFADDRS > +#include <ifaddrs.h> > +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, > + bool *obtained, Error **errp); > +#endif > + > typedef struct GuestFileHandle GuestFileHandle; > > GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp); > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index f5b9e5c530..2a172c6492 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -41,20 +41,12 @@ > #endif > #endif > > -#ifdef __FreeBSD__ > -/* > - * The code under HAVE_GETIFADDRS condition can't be compiled in FreeBSD. > - * Fix it in one of the following patches. > - */ > -#undef HAVE_GETIFADDRS > -#endif > - > #ifdef HAVE_GETIFADDRS > #include <arpa/inet.h> > #include <sys/socket.h> > #include <net/if.h> > +#include <net/ethernet.h> > #include <sys/types.h> > -#include <ifaddrs.h> > #ifdef CONFIG_SOLARIS > #include <sys/sockio.h> > #endif > @@ -2889,6 +2881,57 @@ static int guest_get_network_stats(const char *name, > return -1; > } > > +#ifndef __FreeBSD__ > +/* > + * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a > + * buffer with ETHER_ADDR_LEN length at least. > + * > + * Returns -1 in case of an error, otherwise 0. "obtained" arguument is > + * true if a MAC address was obtained successful, otherwise false. > + */ > In include/qapi/error.h, we recommend returning a bool for success/failure. +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, > + bool *obtained, Error **errp) > +{ > + struct ifreq ifr; > + int sock; > + > + *obtained = false; > + > + /* we haven't obtained HW address yet */ > + sock = socket(PF_INET, SOCK_STREAM, 0); > + if (sock == -1) { > + error_setg_errno(errp, errno, "failed to create socket"); > + return -1; > + } > + > + memset(&ifr, 0, sizeof(ifr)); > + pstrcpy(ifr.ifr_name, IF_NAMESIZE, ifa->ifa_name); > + if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { > + /* > + * We can't get the hw addr of this interface, but that's not a > + * fatal error. > + */ > + if (errno == EADDRNOTAVAIL) { > + /* The interface doesn't have a hw addr (e.g. loopback). */ > + g_debug("failed to get MAC address of %s: %s", > + ifa->ifa_name, strerror(errno)); > + } else{ > + g_warning("failed to get MAC address of %s: %s", > + ifa->ifa_name, strerror(errno)); > + } > + } else { > +#ifdef CONFIG_SOLARIS > + memcpy(buf, &ifr.ifr_addr.sa_data, ETHER_ADDR_LEN); > +#else > + memcpy(buf, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN); > +#endif > + *obtained = true; > + } > + close(sock); > + return 0; > +} > +#endif /* __FreeBSD__ */ > + > /* > * Build information about guest interfaces > */ > @@ -2909,9 +2952,9 @@ GuestNetworkInterfaceList > *qmp_guest_network_get_interfaces(Error **errp) > GuestNetworkInterfaceStat *interface_stat = NULL; > char addr4[INET_ADDRSTRLEN]; > char addr6[INET6_ADDRSTRLEN]; > - int sock; > - struct ifreq ifr; > - unsigned char *mac_addr; > + unsigned char mac_addr[ETHER_ADDR_LEN]; > + int ret; > + bool obtained; > void *p; > > g_debug("Processing %s interface", ifa->ifa_name); > @@ -2926,45 +2969,18 @@ GuestNetworkInterfaceList > *qmp_guest_network_get_interfaces(Error **errp) > } > > if (!info->has_hardware_address) { > - /* we haven't obtained HW address yet */ > - sock = socket(PF_INET, SOCK_STREAM, 0); > - if (sock == -1) { > - error_setg_errno(errp, errno, "failed to create socket"); > + ret = guest_get_hw_addr(ifa, mac_addr, &obtained, errp); > + if (ret == -1) { > goto error; > } > - > - memset(&ifr, 0, sizeof(ifr)); > - pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name); > - if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { > - /* > - * We can't get the hw addr of this interface, but that's > not a > - * fatal error. Don't set info->hardware_address, but keep > - * going. > - */ > - if (errno == EADDRNOTAVAIL) { > - /* The interface doesn't have a hw addr (e.g. > loopback). */ > - g_debug("failed to get MAC address of %s: %s", > - ifa->ifa_name, strerror(errno)); > - } else{ > - g_warning("failed to get MAC address of %s: %s", > - ifa->ifa_name, strerror(errno)); > - } > - > - } else { > -#ifdef CONFIG_SOLARIS > - mac_addr = (unsigned char *) &ifr.ifr_addr.sa_data; > -#else > - mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data; > -#endif > + if (obtained) { > info->hardware_address = > g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x", > (int) mac_addr[0], (int) mac_addr[1], > (int) mac_addr[2], (int) mac_addr[3], > (int) mac_addr[4], (int) mac_addr[5]); > - > info->has_hardware_address = true; > } > - close(sock); > } > > if (ifa->ifa_addr && > -- > 2.34.1 > > otherwise, lgtm Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
On Mon, Oct 3, 2022 at 12:58 PM Marc-André Lureau < marcandre.lureau@gmail.com> wrote: > Hi > > On Mon, Oct 3, 2022 at 1:39 PM Alexander Ivanov < > alexander.ivanov@virtuozzo.com> wrote: > >> In the next patch FreeBSD support for guest-network-get-interfaces will be >> added. Previously move Linux-specific code of HW address getting to a >> separate functions and add a dumb function to commands-bsd.c. >> >> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> >> --- >> qga/commands-bsd.c | 16 +++++++ >> qga/commands-common.h | 6 +++ >> qga/commands-posix.c | 100 ++++++++++++++++++++++++------------------ >> 3 files changed, 80 insertions(+), 42 deletions(-) >> >> diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c >> index ca06692179..40f7ec7600 100644 >> --- a/qga/commands-bsd.c >> +++ b/qga/commands-bsd.c >> @@ -167,3 +167,19 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error >> **errp) >> return NULL; >> } >> #endif /* CONFIG_FSFREEZE */ >> + >> +#ifdef HAVE_GETIFADDRS >> +/* >> + * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a >> + * buffer with ETHER_ADDR_LEN length at least. >> + * >> + * Returns -1 in case of an error, otherwise 0. "obtained" arguument is >> + * true if a MAC address was obtained successful, otherwise false. >> + */ >> +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, >> + bool *obtained, Error **errp) >> +{ >> + *obtained = false; >> + return 0; >> +} >> +#endif /* HAVE_GETIFADDRS */ >> diff --git a/qga/commands-common.h b/qga/commands-common.h >> index 2d9878a634..a9cdaf7278 100644 >> --- a/qga/commands-common.h >> +++ b/qga/commands-common.h >> @@ -56,6 +56,12 @@ int64_t qmp_guest_fsfreeze_do_freeze_list(bool >> has_mountpoints, >> int qmp_guest_fsfreeze_do_thaw(Error **errp); >> #endif /* CONFIG_FSFREEZE */ >> >> +#ifdef HAVE_GETIFADDRS >> +#include <ifaddrs.h> >> +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, >> + bool *obtained, Error **errp); >> +#endif >> + >> typedef struct GuestFileHandle GuestFileHandle; >> >> GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp); >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> index f5b9e5c530..2a172c6492 100644 >> --- a/qga/commands-posix.c >> +++ b/qga/commands-posix.c >> @@ -41,20 +41,12 @@ >> #endif >> #endif >> >> -#ifdef __FreeBSD__ >> -/* >> - * The code under HAVE_GETIFADDRS condition can't be compiled in FreeBSD. >> - * Fix it in one of the following patches. >> - */ >> -#undef HAVE_GETIFADDRS >> -#endif >> - >> #ifdef HAVE_GETIFADDRS >> #include <arpa/inet.h> >> #include <sys/socket.h> >> #include <net/if.h> >> +#include <net/ethernet.h> >> #include <sys/types.h> >> -#include <ifaddrs.h> >> #ifdef CONFIG_SOLARIS >> #include <sys/sockio.h> >> #endif >> @@ -2889,6 +2881,57 @@ static int guest_get_network_stats(const char >> *name, >> return -1; >> } >> >> +#ifndef __FreeBSD__ >> +/* >> + * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a >> + * buffer with ETHER_ADDR_LEN length at least. >> + * >> + * Returns -1 in case of an error, otherwise 0. "obtained" arguument is >> + * true if a MAC address was obtained successful, otherwise false. >> + */ >> > > In include/qapi/error.h, we recommend returning a bool for success/failure. > > +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, >> + bool *obtained, Error **errp) >> +{ >> + struct ifreq ifr; >> + int sock; >> + >> + *obtained = false; >> + >> + /* we haven't obtained HW address yet */ >> + sock = socket(PF_INET, SOCK_STREAM, 0); >> + if (sock == -1) { >> + error_setg_errno(errp, errno, "failed to create socket"); >> + return -1; >> + } >> + >> + memset(&ifr, 0, sizeof(ifr)); >> + pstrcpy(ifr.ifr_name, IF_NAMESIZE, ifa->ifa_name); >> + if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { >> + /* >> + * We can't get the hw addr of this interface, but that's not a >> + * fatal error. >> + */ >> + if (errno == EADDRNOTAVAIL) { >> + /* The interface doesn't have a hw addr (e.g. loopback). */ >> + g_debug("failed to get MAC address of %s: %s", >> + ifa->ifa_name, strerror(errno)); >> + } else{ >> + g_warning("failed to get MAC address of %s: %s", >> + ifa->ifa_name, strerror(errno)); >> + } >> + } else { >> +#ifdef CONFIG_SOLARIS >> + memcpy(buf, &ifr.ifr_addr.sa_data, ETHER_ADDR_LEN); >> +#else >> + memcpy(buf, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN); >> +#endif >> + *obtained = true; >> + } >> + close(sock); >> + return 0; >> +} >> +#endif /* __FreeBSD__ */ >> + >> /* >> * Build information about guest interfaces >> */ >> @@ -2909,9 +2952,9 @@ GuestNetworkInterfaceList >> *qmp_guest_network_get_interfaces(Error **errp) >> GuestNetworkInterfaceStat *interface_stat = NULL; >> char addr4[INET_ADDRSTRLEN]; >> char addr6[INET6_ADDRSTRLEN]; >> - int sock; >> - struct ifreq ifr; >> - unsigned char *mac_addr; >> + unsigned char mac_addr[ETHER_ADDR_LEN]; >> + int ret; >> + bool obtained; >> void *p; >> >> g_debug("Processing %s interface", ifa->ifa_name); >> @@ -2926,45 +2969,18 @@ GuestNetworkInterfaceList >> *qmp_guest_network_get_interfaces(Error **errp) >> } >> >> if (!info->has_hardware_address) { >> - /* we haven't obtained HW address yet */ >> - sock = socket(PF_INET, SOCK_STREAM, 0); >> - if (sock == -1) { >> - error_setg_errno(errp, errno, "failed to create socket"); >> + ret = guest_get_hw_addr(ifa, mac_addr, &obtained, errp); >> + if (ret == -1) { >> goto error; >> } >> - >> - memset(&ifr, 0, sizeof(ifr)); >> - pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name); >> - if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { >> - /* >> - * We can't get the hw addr of this interface, but >> that's not a >> - * fatal error. Don't set info->hardware_address, but >> keep >> - * going. >> - */ >> - if (errno == EADDRNOTAVAIL) { >> - /* The interface doesn't have a hw addr (e.g. >> loopback). */ >> - g_debug("failed to get MAC address of %s: %s", >> - ifa->ifa_name, strerror(errno)); >> - } else{ >> - g_warning("failed to get MAC address of %s: %s", >> - ifa->ifa_name, strerror(errno)); >> - } >> - >> - } else { >> -#ifdef CONFIG_SOLARIS >> - mac_addr = (unsigned char *) &ifr.ifr_addr.sa_data; >> -#else >> - mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data; >> -#endif >> + if (obtained) { >> info->hardware_address = >> g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x", >> (int) mac_addr[0], (int) mac_addr[1], >> (int) mac_addr[2], (int) mac_addr[3], >> (int) mac_addr[4], (int) >> mac_addr[5]); >> - >> info->has_hardware_address = true; >> } >> - close(sock); >> } >> >> if (ifa->ifa_addr && >> -- >> 2.34.1 >> >> > otherwise, lgtm > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > in win32 parts, we have similar behavior, so lgtm Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com> > > > -- > Marc-André Lureau >
diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c index ca06692179..40f7ec7600 100644 --- a/qga/commands-bsd.c +++ b/qga/commands-bsd.c @@ -167,3 +167,19 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) return NULL; } #endif /* CONFIG_FSFREEZE */ + +#ifdef HAVE_GETIFADDRS +/* + * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a + * buffer with ETHER_ADDR_LEN length at least. + * + * Returns -1 in case of an error, otherwise 0. "obtained" arguument is + * true if a MAC address was obtained successful, otherwise false. + */ +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, + bool *obtained, Error **errp) +{ + *obtained = false; + return 0; +} +#endif /* HAVE_GETIFADDRS */ diff --git a/qga/commands-common.h b/qga/commands-common.h index 2d9878a634..a9cdaf7278 100644 --- a/qga/commands-common.h +++ b/qga/commands-common.h @@ -56,6 +56,12 @@ int64_t qmp_guest_fsfreeze_do_freeze_list(bool has_mountpoints, int qmp_guest_fsfreeze_do_thaw(Error **errp); #endif /* CONFIG_FSFREEZE */ +#ifdef HAVE_GETIFADDRS +#include <ifaddrs.h> +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, + bool *obtained, Error **errp); +#endif + typedef struct GuestFileHandle GuestFileHandle; GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp); diff --git a/qga/commands-posix.c b/qga/commands-posix.c index f5b9e5c530..2a172c6492 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -41,20 +41,12 @@ #endif #endif -#ifdef __FreeBSD__ -/* - * The code under HAVE_GETIFADDRS condition can't be compiled in FreeBSD. - * Fix it in one of the following patches. - */ -#undef HAVE_GETIFADDRS -#endif - #ifdef HAVE_GETIFADDRS #include <arpa/inet.h> #include <sys/socket.h> #include <net/if.h> +#include <net/ethernet.h> #include <sys/types.h> -#include <ifaddrs.h> #ifdef CONFIG_SOLARIS #include <sys/sockio.h> #endif @@ -2889,6 +2881,57 @@ static int guest_get_network_stats(const char *name, return -1; } +#ifndef __FreeBSD__ +/* + * Fill "buf" with MAC address by ifaddrs. Pointer buf must point to a + * buffer with ETHER_ADDR_LEN length at least. + * + * Returns -1 in case of an error, otherwise 0. "obtained" arguument is + * true if a MAC address was obtained successful, otherwise false. + */ +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf, + bool *obtained, Error **errp) +{ + struct ifreq ifr; + int sock; + + *obtained = false; + + /* we haven't obtained HW address yet */ + sock = socket(PF_INET, SOCK_STREAM, 0); + if (sock == -1) { + error_setg_errno(errp, errno, "failed to create socket"); + return -1; + } + + memset(&ifr, 0, sizeof(ifr)); + pstrcpy(ifr.ifr_name, IF_NAMESIZE, ifa->ifa_name); + if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { + /* + * We can't get the hw addr of this interface, but that's not a + * fatal error. + */ + if (errno == EADDRNOTAVAIL) { + /* The interface doesn't have a hw addr (e.g. loopback). */ + g_debug("failed to get MAC address of %s: %s", + ifa->ifa_name, strerror(errno)); + } else{ + g_warning("failed to get MAC address of %s: %s", + ifa->ifa_name, strerror(errno)); + } + } else { +#ifdef CONFIG_SOLARIS + memcpy(buf, &ifr.ifr_addr.sa_data, ETHER_ADDR_LEN); +#else + memcpy(buf, &ifr.ifr_hwaddr.sa_data, ETHER_ADDR_LEN); +#endif + *obtained = true; + } + close(sock); + return 0; +} +#endif /* __FreeBSD__ */ + /* * Build information about guest interfaces */ @@ -2909,9 +2952,9 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) GuestNetworkInterfaceStat *interface_stat = NULL; char addr4[INET_ADDRSTRLEN]; char addr6[INET6_ADDRSTRLEN]; - int sock; - struct ifreq ifr; - unsigned char *mac_addr; + unsigned char mac_addr[ETHER_ADDR_LEN]; + int ret; + bool obtained; void *p; g_debug("Processing %s interface", ifa->ifa_name); @@ -2926,45 +2969,18 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) } if (!info->has_hardware_address) { - /* we haven't obtained HW address yet */ - sock = socket(PF_INET, SOCK_STREAM, 0); - if (sock == -1) { - error_setg_errno(errp, errno, "failed to create socket"); + ret = guest_get_hw_addr(ifa, mac_addr, &obtained, errp); + if (ret == -1) { goto error; } - - memset(&ifr, 0, sizeof(ifr)); - pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name); - if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) { - /* - * We can't get the hw addr of this interface, but that's not a - * fatal error. Don't set info->hardware_address, but keep - * going. - */ - if (errno == EADDRNOTAVAIL) { - /* The interface doesn't have a hw addr (e.g. loopback). */ - g_debug("failed to get MAC address of %s: %s", - ifa->ifa_name, strerror(errno)); - } else{ - g_warning("failed to get MAC address of %s: %s", - ifa->ifa_name, strerror(errno)); - } - - } else { -#ifdef CONFIG_SOLARIS - mac_addr = (unsigned char *) &ifr.ifr_addr.sa_data; -#else - mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data; -#endif + if (obtained) { info->hardware_address = g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x", (int) mac_addr[0], (int) mac_addr[1], (int) mac_addr[2], (int) mac_addr[3], (int) mac_addr[4], (int) mac_addr[5]); - info->has_hardware_address = true; } - close(sock); } if (ifa->ifa_addr &&
In the next patch FreeBSD support for guest-network-get-interfaces will be added. Previously move Linux-specific code of HW address getting to a separate functions and add a dumb function to commands-bsd.c. Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- qga/commands-bsd.c | 16 +++++++ qga/commands-common.h | 6 +++ qga/commands-posix.c | 100 ++++++++++++++++++++++++------------------ 3 files changed, 80 insertions(+), 42 deletions(-)