qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 6/7] qga: Move HW address getting to a separate function


From: Alexander Ivanov
Subject: Re: [PATCH 6/7] qga: Move HW address getting to a separate function
Date: Thu, 29 Sep 2022 16:33:44 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0


On 29.09.2022 13:28, Marc-André Lureau wrote:
Hi

On Thu, Sep 29, 2022 at 12:02 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    |  18 +++++++
     qga/commands-common.h |   6 +++
     qga/commands-posix.c  | 114
    +++++++++++++++++++++++-------------------
     3 files changed, 87 insertions(+), 51 deletions(-)

    diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c
    index ca06692179..ca81114171 100644
    --- a/qga/commands-bsd.c
    +++ b/qga/commands-bsd.c
    @@ -167,3 +167,21 @@ 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, 0 if MAC address can't be
    obtained or
    + * 1 if MAC addres is obtained.


Not a typical Error function return value...

Eventually, you could return a bool for error/ok and take an additional "bool *obtained/valid" argument. Just a suggestion.
Got it.

    + */
    +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf,
    Error **errp)
    +{
    +    /*
    +     * We can't get the hw addr of this interface,
    +     * but that's not a fatal error.
    +     */
    +    return 0;
    +}
    +#endif /* HAVE_GETIFADDRS */
    diff --git a/qga/commands-common.h b/qga/commands-common.h
    index 2d9878a634..2485a037fd 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,
    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 6ce894ca6e..0bebd9e690 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
    @@ -2659,14 +2651,6 @@ int64_t
    qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
         return -1;
     }

    -void qmp_guest_set_user_password(const char *username,
    -                                 const char *password,
    -                                 bool crypted,
    -                                 Error **errp)
    -{
    -    error_setg(errp, QERR_UNSUPPORTED);
    -}
    -


Why this in this patch?
Something went wrong when I re-created patches. My bad. Will fix it.

     GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
     {
         error_setg(errp, QERR_UNSUPPORTED);
    @@ -2804,7 +2788,15 @@ out:
             close(datafd[1]);
         }
     }
    -#endif
    +#else /* __linux__ || __FreeBSD__ */
    +void qmp_guest_set_user_password(const char *username,
    +                                 const char *password,
    +                                 bool crypted,
    +                                 Error **errp)
    +{
    +    error_setg(errp, QERR_UNSUPPORTED);
    +}
    +#endif /* __linux__ || __FreeBSD__ */

     #ifdef HAVE_GETIFADDRS
     static GuestNetworkInterface *
    @@ -2887,6 +2879,54 @@ 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, 0 if MAC address can't be
    obtained or
    + * 1 if MAC addres is obtained.
    + */
    +int guest_get_hw_addr(struct ifaddrs *ifa, unsigned char *buf,
    Error **errp)
    +{
    +    struct ifreq ifr;
    +    int sock;
    +
    +    /* 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. 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));
    +        }
    +        close(sock);
    +        return 0;
    +    }
    +#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
    +    close(sock);
    +    return 1;
    +}
    +#endif /* __FreeBSD__ */
    +
     /*
      * Build information about guest interfaces
      */
    @@ -2907,10 +2947,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];
             void *p;
    +        int ret;

             g_debug("Processing %s interface", ifa->ifa_name);

    @@ -2924,45 +2963,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, 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 (ret == 1) {
                     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



looks ok to me otherwise

--
Marc-André Lureau



reply via email to

[Prev in Thread] Current Thread [Next in Thread]