From: marcandre.lureau@redhat.com
To: qemu-devel@nongnu.org
Cc: Richard Henderson <richard.henderson@linaro.org>,
Michael Roth <michael.roth@amd.com>,
Peter Maydell <peter.maydell@linaro.org>,
Andrew Deason <adeason@sinenomine.net>
Subject: [PULL 2/7] qga/commands-posix: Fix iface hw address detection
Date: Wed, 4 May 2022 14:00:56 +0400 [thread overview]
Message-ID: <20220504100101.564747-3-marcandre.lureau@redhat.com> (raw)
In-Reply-To: <20220504100101.564747-1-marcandre.lureau@redhat.com>
From: Andrew Deason <adeason@sinenomine.net>
Since its introduction in commit 3424fc9f16a1 ("qemu-ga: add
guest-network-get-interfaces command"), guest-network-get-interfaces
seems to check if a given interface has a hardware address by checking
'ifa->ifa_flags & SIOCGIFHWADDR'. But ifa_flags is a field for IFF_*
flags (IFF_UP, IFF_LOOPBACK, etc), and comparing it to an ioctl like
SIOCGIFHWADDR doesn't make sense.
On Linux, this isn't a big deal, since SIOCGIFHWADDR has so many bits
set (0x8927), 'ifa->ifa_flags & SIOCGIFHWADDR' will usually have a
nonzero result for any 'normal'-looking interfaces: anything with
IFF_UP (0x1) or IFF_BROADCAST (0x2) set, as well as several
less-common flags. This means we'll try to get the hardware address
for most/all interfaces, even those that don't really have one (like
the loopback device). For those interfaces, Linux just returns a
hardware address of all zeroes.
On Solaris, however, trying to get the hardware address for a loopback
device returns an EADDRNOTAVAIL error. This causes us to return an
error and the entire guest-network-get-interfaces call fails.
Change this logic to always try to get the hardware address for each
interface, and don't return an error if we fail to get it. Instead,
just don't include the 'hardware-address' field in the result if we
can't get the hardware address.
Signed-off-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Message-Id: <20220426195526.7699-3-adeason@sinenomine.net>
---
qga/commands-posix.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 8fc56f7d71ac..febb2ef0ffd6 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2861,7 +2861,7 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
QAPI_LIST_APPEND(tail, info);
}
- if (!info->has_hardware_address && ifa->ifa_flags & SIOCGIFHWADDR) {
+ if (!info->has_hardware_address) {
/* we haven't obtained HW address yet */
sock = socket(PF_INET, SOCK_STREAM, 0);
if (sock == -1) {
@@ -2872,23 +2872,32 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
memset(&ifr, 0, sizeof(ifr));
pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) {
- error_setg_errno(errp, errno,
- "failed to get MAC address of %s",
- ifa->ifa_name);
- close(sock);
- goto error;
- }
+ /*
+ * 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);
- mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;
+ } else {
+ mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;
- 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->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;
+ info->has_hardware_address = true;
+ }
+ close(sock);
}
if (ifa->ifa_addr &&
--
2.36.0.44.g0f828332d5ac
next prev parent reply other threads:[~2022-05-04 11:06 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-04 10:00 [PULL 0/7] QGA patches marcandre.lureau
2022-05-04 10:00 ` [PULL 1/7] qga/commands-posix: Use getifaddrs when available marcandre.lureau
2022-05-04 10:00 ` marcandre.lureau [this message]
2022-05-04 10:00 ` [PULL 3/7] qga/commands-posix: Fix listing ifaces for Solaris marcandre.lureau
2022-05-04 10:00 ` [PULL 4/7] qga/commands-posix: Log all net stats failures marcandre.lureau
2022-05-04 10:00 ` [PULL 5/7] qga/commands-posix: 'guest-shutdown' for Solaris marcandre.lureau
2022-05-04 10:01 ` [PULL 6/7] qga: Introduce NVMe disk bus type marcandre.lureau
2022-05-04 10:01 ` [PULL 7/7] qga: Introduce disk smart marcandre.lureau
2022-05-04 15:06 ` [PULL 0/7] QGA patches Richard Henderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220504100101.564747-3-marcandre.lureau@redhat.com \
--to=marcandre.lureau@redhat.com \
--cc=adeason@sinenomine.net \
--cc=michael.roth@amd.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).