[PATCH 0/4] hyperv: Implement virDomainInterfaceAddresses()
*** BLURB HERE *** Michal Prívozník (4): datatypes: Declare autofree func for virDomainInterface type virsocketaddr: Introduce virSocketAddrSubnetToPrefix() hyperv: Move MAC parsing into a separate function hyperv: Implement virDomainInterfaceAddresses() src/datatypes.h | 2 + src/hyperv/hyperv_driver.c | 228 +++++++++++++++++++++++--- src/hyperv/hyperv_wmi_generator.input | 12 ++ src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 51 ++++++ src/util/virsocketaddr.h | 2 + tests/sockettest.c | 29 ++++ 7 files changed, 300 insertions(+), 25 deletions(-) -- 2.52.0
From: Michal Privoznik <mprivozn@redhat.com> The virDomainInterface type (struct _virDomainInterface) is defined in our public header and even has a public free function (virDomainInterfaceFree()). But in our code we will want to use automatic memory freeing for it. Hence, make appropriate declaration in datatypes.h. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/datatypes.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/datatypes.h b/src/datatypes.h index c5a7ece786..5d543847e9 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -804,6 +804,8 @@ struct _virNWFilterBinding { G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNWFilterBinding, virObjectUnref); +/* virDomainInterface is defined in the public API - libvirt-domain.h */ +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainInterface, virDomainInterfaceFree); /* * Helper APIs for allocating new object instances -- 2.52.0
On Fri, Feb 06, 2026 at 14:52:34 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The virDomainInterface type (struct _virDomainInterface) is defined in our public header and even has a public free function (virDomainInterfaceFree()). But in our code we will want to use automatic memory freeing for it. Hence, make appropriate declaration in datatypes.h.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/datatypes.h | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
From: Michal Privoznik <mprivozn@redhat.com> The aim of this helper is to convert subnet mask to prefix. For instance for input "255.0.0.0" to return 0. Additionally, if the input string is already a prefix (with optional leading slash character) just return that number parsed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ tests/sockettest.c | 29 +++++++++++++++++++++++ 4 files changed, 83 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d81b30f0b6..035eccf70a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3461,6 +3461,7 @@ virSocketAddrSetIPv4AddrNetOrder; virSocketAddrSetIPv6Addr; virSocketAddrSetIPv6AddrNetOrder; virSocketAddrSetPort; +virSocketAddrSubnetToPrefix; # util/virstoragefile.h diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 1f203fb50d..89d41d7656 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -21,6 +21,7 @@ #include "virsocketaddr.h" #include "virerror.h" #include "virbuffer.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -1263,6 +1264,56 @@ virSocketAddrNumericFamily(const char *address) return family; } +/** + * virSocketAddrSubnetToPrefix: + * @subnet: address to convert + * + * Converts subnet mask to prefix. If @subnet is of an IPv4 + * format (NNN.NNN.NNN.NNN) then corresponding prefix length is + * returned (i.e. number of leading bits.). If @subnet is just a + * number (optionally prefixed with '/') then the number is + * parsed and returned. + * + * Returns: prefix corresponding to @subnet, + * -1 otherwise. + */ +int +virSocketAddrSubnetToPrefix(const char *subnet) +{ + struct addrinfo *ai = NULL; + unsigned int prefix = 0; + struct sockaddr_in in; + int ret = -1; + + if (*subnet == '/') { + /* /NN format */ + if (virStrToLong_ui(subnet + 1, NULL, 10, &prefix) < 0) + return -1; + return prefix; + } + + if (virStrToLong_ui(subnet, NULL, 10, &prefix) >= 0) { + /* plain NN format */ + return prefix; + } + + if (virSocketAddrParseInternal(&ai, subnet, AF_INET, AI_NUMERICHOST, false) < 0) + return -1; + + if (ai->ai_family != AF_INET) { + /* huh? */ + goto cleanup; + } + + memcpy(&in, ai->ai_addr, sizeof(in)); + prefix = __builtin_popcount(in.sin_addr.s_addr); + + ret = prefix; + cleanup: + freeaddrinfo(ai); + return ret; +} + /** * virSocketAddrIsNumericLocalhost: * @address: address to check diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index c7ad3250e0..dc6373793b 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -135,6 +135,8 @@ bool virSocketAddrIsWildcard(const virSocketAddr *addr); int virSocketAddrNumericFamily(const char *address); +int virSocketAddrSubnetToPrefix(const char *subnet); + bool virSocketAddrIsNumericLocalhost(const char *addr); int virSocketAddrPTRDomain(const virSocketAddr *addr, diff --git a/tests/sockettest.c b/tests/sockettest.c index 5cb8a9fb72..df62dc6f3b 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -257,6 +257,21 @@ testIsLocalhostHelper(const void *opaque) return 0; } +struct testSubnetToPrefixData { + const char *addr; + int prefix; +}; + +static int +testSubnetToPrefixHelper(const void *opaque) +{ + const struct testSubnetToPrefixData *data = opaque; + + if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix) + return -1; + return 0; +} + static int mymain(void) { @@ -352,6 +367,14 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_SUBNET_TO_PREFIX(addr, prefix) \ + do { \ + struct testSubnetToPrefixData data = { addr, prefix }; \ + if (virTestRun("Test subnet to prefix " addr, \ + testSubnetToPrefixHelper, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_UNSPEC, true); DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_INET, true); DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_INET6, false); @@ -476,6 +499,12 @@ mymain(void) DO_TEST_LOCALHOST("hello", false); DO_TEST_LOCALHOST("fe80::1:1", false); + DO_TEST_SUBNET_TO_PREFIX("0.0.0.0", 0); + DO_TEST_SUBNET_TO_PREFIX("255.0.0.0", 8); + DO_TEST_SUBNET_TO_PREFIX("255.255.255.254", 31); + DO_TEST_SUBNET_TO_PREFIX("64", 64); + DO_TEST_SUBNET_TO_PREFIX("/64", 64); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.52.0
On 2/6/26 7:52 AM, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The aim of this helper is to convert subnet mask to prefix. For instance for input "255.0.0.0" to return 0. Additionally, if the
Just FYI, this comment doesn't seem to match the implementation or the tests. You have a test defined for 255.0.0.0 where the expected return value is 8, which makes sense.
input string is already a prefix (with optional leading slash character) just return that number parsed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ tests/sockettest.c | 29 +++++++++++++++++++++++ 4 files changed, 83 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d81b30f0b6..035eccf70a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3461,6 +3461,7 @@ virSocketAddrSetIPv4AddrNetOrder; virSocketAddrSetIPv6Addr; virSocketAddrSetIPv6AddrNetOrder; virSocketAddrSetPort; +virSocketAddrSubnetToPrefix;
# util/virstoragefile.h diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 1f203fb50d..89d41d7656 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -21,6 +21,7 @@ #include "virsocketaddr.h" #include "virerror.h" #include "virbuffer.h" +#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -1263,6 +1264,56 @@ virSocketAddrNumericFamily(const char *address) return family; }
+/** + * virSocketAddrSubnetToPrefix: + * @subnet: address to convert + * + * Converts subnet mask to prefix. If @subnet is of an IPv4 + * format (NNN.NNN.NNN.NNN) then corresponding prefix length is + * returned (i.e. number of leading bits.). If @subnet is just a + * number (optionally prefixed with '/') then the number is + * parsed and returned. + * + * Returns: prefix corresponding to @subnet, + * -1 otherwise. + */ +int +virSocketAddrSubnetToPrefix(const char *subnet) +{ + struct addrinfo *ai = NULL; + unsigned int prefix = 0; + struct sockaddr_in in; + int ret = -1; + + if (*subnet == '/') { + /* /NN format */ + if (virStrToLong_ui(subnet + 1, NULL, 10, &prefix) < 0) + return -1; + return prefix; + } + + if (virStrToLong_ui(subnet, NULL, 10, &prefix) >= 0) { + /* plain NN format */ + return prefix; + } + + if (virSocketAddrParseInternal(&ai, subnet, AF_INET, AI_NUMERICHOST, false) < 0) + return -1; + + if (ai->ai_family != AF_INET) { + /* huh? */ + goto cleanup; + } + + memcpy(&in, ai->ai_addr, sizeof(in)); + prefix = __builtin_popcount(in.sin_addr.s_addr); + + ret = prefix; + cleanup: + freeaddrinfo(ai); + return ret; +} + /** * virSocketAddrIsNumericLocalhost: * @address: address to check diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index c7ad3250e0..dc6373793b 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -135,6 +135,8 @@ bool virSocketAddrIsWildcard(const virSocketAddr *addr);
int virSocketAddrNumericFamily(const char *address);
+int virSocketAddrSubnetToPrefix(const char *subnet); + bool virSocketAddrIsNumericLocalhost(const char *addr);
int virSocketAddrPTRDomain(const virSocketAddr *addr, diff --git a/tests/sockettest.c b/tests/sockettest.c index 5cb8a9fb72..df62dc6f3b 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -257,6 +257,21 @@ testIsLocalhostHelper(const void *opaque) return 0; }
+struct testSubnetToPrefixData { + const char *addr; + int prefix; +}; + +static int +testSubnetToPrefixHelper(const void *opaque) +{ + const struct testSubnetToPrefixData *data = opaque; + + if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix) + return -1; + return 0; +} + static int mymain(void) { @@ -352,6 +367,14 @@ mymain(void) ret = -1; \ } while (0)
+#define DO_TEST_SUBNET_TO_PREFIX(addr, prefix) \ + do { \ + struct testSubnetToPrefixData data = { addr, prefix }; \ + if (virTestRun("Test subnet to prefix " addr, \ + testSubnetToPrefixHelper, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_UNSPEC, true); DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_INET, true); DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_INET6, false); @@ -476,6 +499,12 @@ mymain(void) DO_TEST_LOCALHOST("hello", false); DO_TEST_LOCALHOST("fe80::1:1", false);
+ DO_TEST_SUBNET_TO_PREFIX("0.0.0.0", 0); + DO_TEST_SUBNET_TO_PREFIX("255.0.0.0", 8); + DO_TEST_SUBNET_TO_PREFIX("255.255.255.254", 31); + DO_TEST_SUBNET_TO_PREFIX("64", 64); + DO_TEST_SUBNET_TO_PREFIX("/64", 64); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }
On 2/6/26 16:03, Jonathon Jongsma wrote:
On 2/6/26 7:52 AM, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The aim of this helper is to convert subnet mask to prefix. For instance for input "255.0.0.0" to return 0. Additionally, if the
Just FYI, this comment doesn't seem to match the implementation or the tests. You have a test defined for 255.0.0.0 where the expected return value is 8, which makes sense.
Ooops, yes. A typo. Fixed locally. Thanks! Michal
On Fri, Feb 06, 2026 at 14:52:35 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The aim of this helper is to convert subnet mask to prefix. For instance for input "255.0.0.0" to return 0. Additionally, if the input string is already a prefix (with optional leading slash character) just return that number parsed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ tests/sockettest.c | 29 +++++++++++++++++++++++ 4 files changed, 83 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d81b30f0b6..035eccf70a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3461,6 +3461,7 @@ virSocketAddrSetIPv4AddrNetOrder; virSocketAddrSetIPv6Addr; virSocketAddrSetIPv6AddrNetOrder; virSocketAddrSetPort; +virSocketAddrSubnetToPrefix;
# util/virstoragefile.h diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 1f203fb50d..89d41d7656 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -21,6 +21,7 @@ #include "virsocketaddr.h" #include "virerror.h" #include "virbuffer.h" +#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -1263,6 +1264,56 @@ virSocketAddrNumericFamily(const char *address) return family; }
+/** + * virSocketAddrSubnetToPrefix: + * @subnet: address to convert + * + * Converts subnet mask to prefix. If @subnet is of an IPv4 + * format (NNN.NNN.NNN.NNN) then corresponding prefix length is + * returned (i.e. number of leading bits.). If @subnet is just a + * number (optionally prefixed with '/') then the number is + * parsed and returned.
This doesn't say any caveats if e.g. the IPv4 string you pass in isn't a subnet mask at all ...
+ * Returns: prefix corresponding to @subnet, + * -1 otherwise. + */ +int +virSocketAddrSubnetToPrefix(const char *subnet) +{ + struct addrinfo *ai = NULL; + unsigned int prefix = 0; + struct sockaddr_in in; + int ret = -1; + + if (*subnet == '/') { + /* /NN format */ + if (virStrToLong_ui(subnet + 1, NULL, 10, &prefix) < 0) + return -1; + return prefix; + } + + if (virStrToLong_ui(subnet, NULL, 10, &prefix) >= 0) { + /* plain NN format */ + return prefix; + } + + if (virSocketAddrParseInternal(&ai, subnet, AF_INET, AI_NUMERICHOST, false) < 0) + return -1; + + if (ai->ai_family != AF_INET) { + /* huh? */ + goto cleanup; + } + + memcpy(&in, ai->ai_addr, sizeof(in)); + prefix = __builtin_popcount(in.sin_addr.s_addr);
... so if you don't pass a mask it gives not really documented answers. With the impl above the following predicates of the test added later are also true: DO_TEST_SUBNET_TO_PREFIX("192.168.0.1", 6); DO_TEST_SUBNET_TO_PREFIX("129.0.0.1", 3); Depending on the expectations either document the expectation of what happens argument isn't a subnet or add some checks E.g. you could use __builtin_ffs to figure out the number of leading bits and then compare with __builtin_popcount to see if other bits aren't set.
diff --git a/tests/sockettest.c b/tests/sockettest.c index 5cb8a9fb72..df62dc6f3b 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -257,6 +257,21 @@ testIsLocalhostHelper(const void *opaque) return 0; }
+struct testSubnetToPrefixData { + const char *addr; + int prefix; +}; + +static int +testSubnetToPrefixHelper(const void *opaque) +{ + const struct testSubnetToPrefixData *data = opaque; + + if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix) + return -1;
Consider adding: diff --git a/tests/sockettest.c b/tests/sockettest.c index df62dc6f3b..37f654880a 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -266,9 +266,13 @@ static int testSubnetToPrefixHelper(const void *opaque) { const struct testSubnetToPrefixData *data = opaque; + int val = virSocketAddrSubnetToPrefix(data->addr); - if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix) + if (val != data->prefix) { + fprintf(stderr, "actual: '%d', expected: '%d'", val, data->prefix); return -1; + } + return 0; } for debuggability
On 2/10/26 18:10, Peter Krempa wrote:
On Fri, Feb 06, 2026 at 14:52:35 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The aim of this helper is to convert subnet mask to prefix. For instance for input "255.0.0.0" to return 0. Additionally, if the input string is already a prefix (with optional leading slash character) just return that number parsed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ tests/sockettest.c | 29 +++++++++++++++++++++++ 4 files changed, 83 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d81b30f0b6..035eccf70a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3461,6 +3461,7 @@ virSocketAddrSetIPv4AddrNetOrder; virSocketAddrSetIPv6Addr; virSocketAddrSetIPv6AddrNetOrder; virSocketAddrSetPort; +virSocketAddrSubnetToPrefix;
# util/virstoragefile.h diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 1f203fb50d..89d41d7656 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -21,6 +21,7 @@ #include "virsocketaddr.h" #include "virerror.h" #include "virbuffer.h" +#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -1263,6 +1264,56 @@ virSocketAddrNumericFamily(const char *address) return family; }
+/** + * virSocketAddrSubnetToPrefix: + * @subnet: address to convert + * + * Converts subnet mask to prefix. If @subnet is of an IPv4 + * format (NNN.NNN.NNN.NNN) then corresponding prefix length is + * returned (i.e. number of leading bits.). If @subnet is just a + * number (optionally prefixed with '/') then the number is + * parsed and returned.
This doesn't say any caveats if e.g. the IPv4 string you pass in isn't a subnet mask at all ...
+ * Returns: prefix corresponding to @subnet, + * -1 otherwise. + */ +int +virSocketAddrSubnetToPrefix(const char *subnet) +{ + struct addrinfo *ai = NULL; + unsigned int prefix = 0; + struct sockaddr_in in; + int ret = -1; + + if (*subnet == '/') { + /* /NN format */ + if (virStrToLong_ui(subnet + 1, NULL, 10, &prefix) < 0) + return -1; + return prefix; + } + + if (virStrToLong_ui(subnet, NULL, 10, &prefix) >= 0) { + /* plain NN format */ + return prefix; + } + + if (virSocketAddrParseInternal(&ai, subnet, AF_INET, AI_NUMERICHOST, false) < 0) + return -1; + + if (ai->ai_family != AF_INET) { + /* huh? */ + goto cleanup; + } + + memcpy(&in, ai->ai_addr, sizeof(in)); + prefix = __builtin_popcount(in.sin_addr.s_addr);
... so if you don't pass a mask it gives not really documented answers.
With the impl above the following predicates of the test added later are also true:
DO_TEST_SUBNET_TO_PREFIX("192.168.0.1", 6); DO_TEST_SUBNET_TO_PREFIX("129.0.0.1", 3);
Depending on the expectations either document the expectation of what happens argument isn't a subnet or add some checks
E.g. you could use __builtin_ffs to figure out the number of leading bits and then compare with __builtin_popcount to see if other bits aren't set.
Would this work on big endian machines? I mean, even on my little endian machine 255.255.255.254 is stored as 0xfeffffff. So I guess I'll just update documentation. How about: * parsed and returned. There is a corner case: if @subnet is * valid IPv4 address but not valid subnet mask then a positive * value is returned, but obviously it is not valid prefix.
diff --git a/tests/sockettest.c b/tests/sockettest.c index 5cb8a9fb72..df62dc6f3b 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -257,6 +257,21 @@ testIsLocalhostHelper(const void *opaque) return 0; }
+struct testSubnetToPrefixData { + const char *addr; + int prefix; +}; + +static int +testSubnetToPrefixHelper(const void *opaque) +{ + const struct testSubnetToPrefixData *data = opaque; + + if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix) + return -1;
Consider adding:
diff --git a/tests/sockettest.c b/tests/sockettest.c index df62dc6f3b..37f654880a 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -266,9 +266,13 @@ static int testSubnetToPrefixHelper(const void *opaque) { const struct testSubnetToPrefixData *data = opaque; + int val = virSocketAddrSubnetToPrefix(data->addr);
- if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix) + if (val != data->prefix) { + fprintf(stderr, "actual: '%d', expected: '%d'", val, data->prefix); return -1; + } + return 0; }
for debuggability
Squashed in locally. Michal
On Thu, Feb 12, 2026 at 14:36:36 +0100, Michal Prívozník wrote:
On 2/10/26 18:10, Peter Krempa wrote:
On Fri, Feb 06, 2026 at 14:52:35 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The aim of this helper is to convert subnet mask to prefix. For instance for input "255.0.0.0" to return 0. Additionally, if the input string is already a prefix (with optional leading slash character) just return that number parsed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ tests/sockettest.c | 29 +++++++++++++++++++++++ 4 files changed, 83 insertions(+)
[...]
+ memcpy(&in, ai->ai_addr, sizeof(in)); + prefix = __builtin_popcount(in.sin_addr.s_addr);
... so if you don't pass a mask it gives not really documented answers.
With the impl above the following predicates of the test added later are also true:
DO_TEST_SUBNET_TO_PREFIX("192.168.0.1", 6); DO_TEST_SUBNET_TO_PREFIX("129.0.0.1", 3);
Depending on the expectations either document the expectation of what happens argument isn't a subnet or add some checks
E.g. you could use __builtin_ffs to figure out the number of leading bits and then compare with __builtin_popcount to see if other bits aren't set.
Would this work on big endian machines? I mean, even on my little endian machine 255.255.255.254 is stored as 0xfeffffff. So I guess I'll just update documentation. How about:
I didn't try actually :D
* parsed and returned. There is a corner case: if @subnet is * valid IPv4 address but not valid subnet mask then a positive * value is returned, but obviously it is not valid prefix.
Documenting is fine with me. Alternatively (to the wording above) you can outline accepted strings instead, but what you wrote is okay too. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
From: Michal Privoznik <mprivozn@redhat.com> When constructing a domain definition, NICs are fetched from WMI and their MAC addresses are then parsed. Move this code into a separate function so that it can be reused later. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/hyperv/hyperv_driver.c | 59 ++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 8dd56f39dc..fbc76544df 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1550,18 +1550,48 @@ hypervDomainDefParseSerial(virDomainDef *def, Msvm_ResourceAllocationSettingData } +static int +hypervDomainDefParseEthernetAdapterMAC(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *net, + virMacAddr *mac) +{ + g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL; + char *sepsdPATH = NULL; + g_autofree char *sepsdEscaped = NULL; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + + sepsdPATH = net->data->Parent; + sepsdEscaped = virStringReplace(sepsdPATH, "\\", "\\\\"); + virBufferAsprintf(&query, + MSVM_SYNTHETICETHERNETPORTSETTINGDATA_WQL_SELECT "WHERE __PATH = '%s'", + sepsdEscaped); + + if (hypervGetWmiClass(Msvm_SyntheticEthernetPortSettingData, &sepsd) < 0) + return -1; + + if (!sepsd) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not retrieve NIC settings")); + return -1; + } + + /* set mac address */ + if (virMacAddrParseHex(sepsd->data->Address, mac) < 0) + return -1; + + return 0; +} + + static int hypervDomainDefParseEthernetAdapter(virDomainDef *def, Msvm_EthernetPortAllocationSettingData *net, hypervPrivate *priv) { g_autoptr(virDomainNetDef) ndef = g_new0(virDomainNetDef, 1); - g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL; g_autoptr(Msvm_VirtualEthernetSwitch) vSwitch = NULL; char **switchConnection = NULL; g_autofree char *switchConnectionEscaped = NULL; - char *sepsdPATH = NULL; - g_autofree char *sepsdEscaped = NULL; + g_autofree char *sepsdInstanceIDEscaped = NULL; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID); @@ -1580,28 +1610,7 @@ hypervDomainDefParseEthernetAdapter(virDomainDef *def, return 0; } - /* - * Now we retrieve the associated Msvm_SyntheticEthernetPortSettingData and - * Msvm_VirtualEthernetSwitch objects and use them to build the XML definition. - */ - - /* begin by getting the Msvm_SyntheticEthernetPortSettingData object */ - sepsdPATH = net->data->Parent; - sepsdEscaped = virStringReplace(sepsdPATH, "\\", "\\\\"); - virBufferAsprintf(&query, - MSVM_SYNTHETICETHERNETPORTSETTINGDATA_WQL_SELECT "WHERE __PATH = '%s'", - sepsdEscaped); - - if (hypervGetWmiClass(Msvm_SyntheticEthernetPortSettingData, &sepsd) < 0) - return -1; - - if (!sepsd) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not retrieve NIC settings")); - return -1; - } - - /* set mac address */ - if (virMacAddrParseHex(sepsd->data->Address, &ndef->mac) < 0) + if (hypervDomainDefParseEthernetAdapterMAC(priv, net, &ndef->mac) < 0) return -1; /* now we get the Msvm_VirtualEthernetSwitch */ -- 2.52.0
On Fri, Feb 06, 2026 at 14:52:36 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
When constructing a domain definition, NICs are fetched from WMI and their MAC addresses are then parsed. Move this code into a separate function so that it can be reused later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/hyperv/hyperv_driver.c | 59 ++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 25 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
From: Michal Privoznik <mprivozn@redhat.com> The virDomainInterfaceAddresses() API accepts @source argument, but since this is hyperv, we can't really use _SRC_LEASE (we didn't spawn any dnsmasq there), not _SRC_ARP. The only source that's more or less usable is _SRC_AGENT. Okay, there's no QEMU guest agent running, but hyperv has its own guest agent. In my testing (with Linux guest) I had to install 'hyperv' package and then enable 'hv_kvp_daemon.service'. After that, Msvm_GuestNetworkAdapterConfiguration struct [1] contained guest IP addresses. There's one caveat though: the interface name (virDomainInterface::name). We don't fetch that one even for hypervDomainGetXMLDesc() case. And there's no <target dev=''/> either nor device alias (v12.0.0-43-g4009126f17). So just put InstanceID there for now, which is this long path, with some UUIDs, e.g.: Microsoft:5C58E5F2-946E-490F-B81D-6E2A7328640D\C85554E0-2B3B-487C-A557-D230BFF5F9E6\ But hey, at least it's unique. 1: https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/msvm-guestnetworka... Resolves: https://issues.redhat.com/browse/RHEL-145306 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Admittedly, the 'virsh domifaddr' output is misaligned after this, but I'll post a separate patch for that. src/hyperv/hyperv_driver.c | 169 ++++++++++++++++++++++++++ src/hyperv/hyperv_wmi_generator.input | 12 ++ 2 files changed, 181 insertions(+) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index fbc76544df..c25cb91c13 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -3654,6 +3654,174 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, } +static int +hypervDomainInterfaceAddressesParseOne(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *net, + virDomainInterfacePtr *oneIfaceRet) +{ + g_autoptr(virDomainInterface) iface = NULL; + g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL; + g_autoptr(Msvm_GuestNetworkAdapterConfiguration) aConfig = NULL; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virMacAddr macAddr = { 0 }; + char macAddrStr[VIR_MAC_STRING_BUFLEN] = { 0 }; + + VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID); + + iface = g_new0(virDomainInterface, 1); + iface->name = g_strdup(net->data->InstanceID); + + if (hypervDomainDefParseEthernetAdapterMAC(priv, net, &macAddr) < 0) + return -1; + + iface->hwaddr = g_strdup(virMacAddrFormat(&macAddr, macAddrStr)); + + virBufferAsprintf(&query, + "ASSOCIATORS OF {%s} " + "WHERE AssocClass=Msvm_SettingDataComponent " + "ResultClass=Msvm_GuestNetworkAdapterConfiguration", + net->data->Parent); + + if (hypervGetWmiClass(Msvm_GuestNetworkAdapterConfiguration, &aConfig) < 0) + return -1; + + if (aConfig) { + size_t nAddr = aConfig->data->IPAddresses.count; + size_t i; + + if (aConfig->data->Subnets.count != nAddr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("the number of IP addresses (%1$zu) does not match the number of subnets (%2$d)"), + nAddr, aConfig->data->Subnets.count); + return -1; + } + + iface->addrs = g_new0(virDomainIPAddress, nAddr); + iface->naddrs = nAddr; + for (i = 0; i < nAddr; i++) { + const char *ipAddrStr = ((const char **) aConfig->data->IPAddresses.data)[i]; + const char *subnetAddrStr = ((const char **) aConfig->data->Subnets.data)[i]; + virDomainIPAddressPtr ip = &iface->addrs[i]; + int family; + int prefix; + + VIR_DEBUG("ipAddrStr='%s' subnetAddrStr='%s'", + ipAddrStr, subnetAddrStr); + + ip->addr = g_strdup(ipAddrStr); + family = virSocketAddrNumericFamily(ipAddrStr); + if (family == AF_INET6) { + ip->type = VIR_IP_ADDR_TYPE_IPV6; + } else if (family == AF_INET) { + ip->type = VIR_IP_ADDR_TYPE_IPV4; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown IP address family of '%1$s'"), + ipAddrStr); + return -1; + } + + prefix = virSocketAddrSubnetToPrefix(subnetAddrStr); + if (prefix < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected subnet mask '%1$s'"), + subnetAddrStr); + return -1; + } + ip->prefix = prefix; + } + } + + *oneIfaceRet = g_steal_pointer(&iface); + return 0; +} + + +static ssize_t +hypervDomainInterfaceAddressesParseList(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *nets, + virDomainInterfacePtr **ifaces) +{ + Msvm_EthernetPortAllocationSettingData *entry = nets; + size_t nifaces = 0; + + while (entry) { + virDomainInterfacePtr oneIface = NULL; + + if (hypervDomainInterfaceAddressesParseOne(priv, entry, &oneIface) < 0) + return -1; + + if (oneIface) + VIR_APPEND_ELEMENT(*ifaces, nifaces, oneIface); + + entry = entry->next; + } + + return nifaces; +} + + +static int +hypervDomainInterfaceAddresses(virDomainPtr dom, + virDomainInterfacePtr **ifaces, + unsigned int source, + unsigned int flags) +{ + hypervPrivate *priv = NULL; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) virtualSystemSettingData = NULL; + g_autoptr(Msvm_EthernetPortAllocationSettingData) nets = NULL; + virDomainInterfacePtr *ifacesRet = NULL; + ssize_t ifacesRetCount = 0; + int ret = -1; + + virCheckFlags(0, -1); + + if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("Unknown IP address data source %1$d"), + source); + return -1; + } + + if (hypervMsvmComputerSystemFromDomain(dom, &computerSystem) < 0) + return -1; + + priv = dom->conn->privateData; + virUUIDFormat(dom->uuid, uuid_string); + + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, + uuid_string, + &virtualSystemSettingData) < 0) { + return -1; + } + + if (hypervGetEthernetPortAllocationSD(priv, + virtualSystemSettingData->data->InstanceID, + &nets) < 0) { + return -1; + } + + ifacesRetCount = hypervDomainInterfaceAddressesParseList(priv, nets, &ifacesRet); + if (ifacesRetCount < 0) + goto cleanup; + + *ifaces = g_steal_pointer(&ifacesRet); + ret = ifacesRetCount; + + cleanup: + if (ret < 0) { + while (ifacesRetCount > 0) { + virDomainInterfaceFree(ifacesRet[--ifacesRetCount]); + } + VIR_FREE(ifacesRet); + } + + return ret; +} + + static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", .connectOpen = hypervConnectOpen, /* 0.9.5 */ @@ -3717,6 +3885,7 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainManagedSaveRemove = hypervDomainManagedSaveRemove, /* 0.9.5 */ .domainSendKey = hypervDomainSendKey, /* 3.6.0 */ .connectIsAlive = hypervConnectIsAlive, /* 0.9.8 */ + .domainInterfaceAddresses = hypervDomainInterfaceAddresses, /* 12.1.0 */ }; diff --git a/src/hyperv/hyperv_wmi_generator.input b/src/hyperv/hyperv_wmi_generator.input index 0b342cbfa6..e6053267f8 100644 --- a/src/hyperv/hyperv_wmi_generator.input +++ b/src/hyperv/hyperv_wmi_generator.input @@ -866,6 +866,18 @@ class Msvm_SyntheticEthernetPortSettingData end +class Msvm_GuestNetworkAdapterConfiguration + string InstanceID + uint16 ProtocolIFType + boolean DHCPEnabled + string IPAddresses[] + string Subnets[] + string DefaultGateways[] + string DNSServers[] + uint16 IPAddressOrigins[] +end + + class Msvm_EthernetPortAllocationSettingData string InstanceID string Caption -- 2.52.0
On Fri, Feb 06, 2026 at 14:52:37 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The virDomainInterfaceAddresses() API accepts @source argument, but since this is hyperv, we can't really use _SRC_LEASE (we didn't spawn any dnsmasq there), not _SRC_ARP. The only source that's more or less usable is _SRC_AGENT. Okay, there's no QEMU guest agent running, but hyperv has its own guest agent. In my testing (with Linux guest) I had to install 'hyperv' package and then enable 'hv_kvp_daemon.service'. After that, Msvm_GuestNetworkAdapterConfiguration struct [1] contained guest IP addresses.
There's one caveat though: the interface name (virDomainInterface::name). We don't fetch that one even for hypervDomainGetXMLDesc() case. And there's no <target dev=''/> either nor device alias (v12.0.0-43-g4009126f17). So just put InstanceID there for now, which is this long path, with some UUIDs, e.g.:
Microsoft:5C58E5F2-946E-490F-B81D-6E2A7328640D\C85554E0-2B3B-487C-A557-D230BFF5F9E6\
But hey, at least it's unique.
1: https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/msvm-guestnetworka... Resolves: https://issues.redhat.com/browse/RHEL-145306 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Admittedly, the 'virsh domifaddr' output is misaligned after this, but I'll post a separate patch for that.
Disclaimer: I have absolutely no idea about the hyperv specific bits.
src/hyperv/hyperv_driver.c | 169 ++++++++++++++++++++++++++ src/hyperv/hyperv_wmi_generator.input | 12 ++ 2 files changed, 181 insertions(+)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index fbc76544df..c25cb91c13 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -3654,6 +3654,174 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, }
+static int +hypervDomainInterfaceAddressesParseOne(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *net, + virDomainInterfacePtr *oneIfaceRet) +{ + g_autoptr(virDomainInterface) iface = NULL; + g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL; + g_autoptr(Msvm_GuestNetworkAdapterConfiguration) aConfig = NULL; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virMacAddr macAddr = { 0 }; + char macAddrStr[VIR_MAC_STRING_BUFLEN] = { 0 }; + + VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID); + + iface = g_new0(virDomainInterface, 1); + iface->name = g_strdup(net->data->InstanceID); + + if (hypervDomainDefParseEthernetAdapterMAC(priv, net, &macAddr) < 0) + return -1; + + iface->hwaddr = g_strdup(virMacAddrFormat(&macAddr, macAddrStr)); + + virBufferAsprintf(&query, + "ASSOCIATORS OF {%s} " + "WHERE AssocClass=Msvm_SettingDataComponent " + "ResultClass=Msvm_GuestNetworkAdapterConfiguration", + net->data->Parent);
'query' is unused after this.
+ + if (hypervGetWmiClass(Msvm_GuestNetworkAdapterConfiguration, &aConfig) < 0) + return -1; + + if (aConfig) { + size_t nAddr = aConfig->data->IPAddresses.count; + size_t i; + + if (aConfig->data->Subnets.count != nAddr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("the number of IP addresses (%1$zu) does not match the number of subnets (%2$d)"), + nAddr, aConfig->data->Subnets.count); + return -1; + } + + iface->addrs = g_new0(virDomainIPAddress, nAddr); + iface->naddrs = nAddr; + for (i = 0; i < nAddr; i++) { + const char *ipAddrStr = ((const char **) aConfig->data->IPAddresses.data)[i]; + const char *subnetAddrStr = ((const char **) aConfig->data->Subnets.data)[i]; + virDomainIPAddressPtr ip = &iface->addrs[i]; + int family; + int prefix; + + VIR_DEBUG("ipAddrStr='%s' subnetAddrStr='%s'", + ipAddrStr, subnetAddrStr); + + ip->addr = g_strdup(ipAddrStr); + family = virSocketAddrNumericFamily(ipAddrStr); + if (family == AF_INET6) { + ip->type = VIR_IP_ADDR_TYPE_IPV6; + } else if (family == AF_INET) { + ip->type = VIR_IP_ADDR_TYPE_IPV4; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown IP address family of '%1$s'"), + ipAddrStr); + return -1; + } + + prefix = virSocketAddrSubnetToPrefix(subnetAddrStr); + if (prefix < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected subnet mask '%1$s'"), + subnetAddrStr); + return -1; + } + ip->prefix = prefix; + } + } + + *oneIfaceRet = g_steal_pointer(&iface); + return 0; +} + + +static ssize_t +hypervDomainInterfaceAddressesParseList(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *nets, + virDomainInterfacePtr **ifaces) +{ + Msvm_EthernetPortAllocationSettingData *entry = nets; + size_t nifaces = 0; + + while (entry) { + virDomainInterfacePtr oneIface = NULL; + + if (hypervDomainInterfaceAddressesParseOne(priv, entry, &oneIface) < 0) + return -1;
If this fails ....
+ + if (oneIface) + VIR_APPEND_ELEMENT(*ifaces, nifaces, oneIface);
... after this was partially filled, the caller has no way of figuring out how many entries to free. This function should cleanup after itself internally.
+ + entry = entry->next; + } + + return nifaces; +} + + +static int +hypervDomainInterfaceAddresses(virDomainPtr dom, + virDomainInterfacePtr **ifaces, + unsigned int source, + unsigned int flags) +{ + hypervPrivate *priv = NULL; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) virtualSystemSettingData = NULL; + g_autoptr(Msvm_EthernetPortAllocationSettingData) nets = NULL; + virDomainInterfacePtr *ifacesRet = NULL; + ssize_t ifacesRetCount = 0; + int ret = -1; + + virCheckFlags(0, -1); + + if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("Unknown IP address data source %1$d"), + source); + return -1; + } + + if (hypervMsvmComputerSystemFromDomain(dom, &computerSystem) < 0) + return -1; + + priv = dom->conn->privateData; + virUUIDFormat(dom->uuid, uuid_string); + + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, + uuid_string, + &virtualSystemSettingData) < 0) { + return -1; + } + + if (hypervGetEthernetPortAllocationSD(priv, + virtualSystemSettingData->data->InstanceID, + &nets) < 0) { + return -1; + } + + ifacesRetCount = hypervDomainInterfaceAddressesParseList(priv, nets, &ifacesRet); + if (ifacesRetCount < 0) + goto cleanup; + + *ifaces = g_steal_pointer(&ifacesRet); + ret = ifacesRetCount; + + cleanup: + if (ret < 0) {
The only case when 'ret' is <0 here is ...
+ while (ifacesRetCount > 0) {
... when this is also -1
+ virDomainInterfaceFree(ifacesRet[--ifacesRetCount]); + }
So all of this is dead code, and also actually due to flawed error reporting in 'hypervDomainInterfaceAddressesParseList' leaks the array if it was partially filled.
+ VIR_FREE(ifacesRet); + } + + return ret; +} + + static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", .connectOpen = hypervConnectOpen, /* 0.9.5 */
I'm wondering what's the point of 'query' above, if it's a leftover from debugging then I'm okay with that. With the cleanup of partially filled data fixed: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On 2/10/26 18:40, Peter Krempa wrote:
On Fri, Feb 06, 2026 at 14:52:37 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The virDomainInterfaceAddresses() API accepts @source argument, but since this is hyperv, we can't really use _SRC_LEASE (we didn't spawn any dnsmasq there), not _SRC_ARP. The only source that's more or less usable is _SRC_AGENT. Okay, there's no QEMU guest agent running, but hyperv has its own guest agent. In my testing (with Linux guest) I had to install 'hyperv' package and then enable 'hv_kvp_daemon.service'. After that, Msvm_GuestNetworkAdapterConfiguration struct [1] contained guest IP addresses.
There's one caveat though: the interface name (virDomainInterface::name). We don't fetch that one even for hypervDomainGetXMLDesc() case. And there's no <target dev=''/> either nor device alias (v12.0.0-43-g4009126f17). So just put InstanceID there for now, which is this long path, with some UUIDs, e.g.:
Microsoft:5C58E5F2-946E-490F-B81D-6E2A7328640D\C85554E0-2B3B-487C-A557-D230BFF5F9E6\
But hey, at least it's unique.
1: https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/msvm-guestnetworka... Resolves: https://issues.redhat.com/browse/RHEL-145306 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Admittedly, the 'virsh domifaddr' output is misaligned after this, but I'll post a separate patch for that.
Disclaimer: I have absolutely no idea about the hyperv specific bits.
src/hyperv/hyperv_driver.c | 169 ++++++++++++++++++++++++++ src/hyperv/hyperv_wmi_generator.input | 12 ++ 2 files changed, 181 insertions(+)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index fbc76544df..c25cb91c13 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -3654,6 +3654,174 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, }
+static int +hypervDomainInterfaceAddressesParseOne(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *net, + virDomainInterfacePtr *oneIfaceRet) +{ + g_autoptr(virDomainInterface) iface = NULL; + g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL; + g_autoptr(Msvm_GuestNetworkAdapterConfiguration) aConfig = NULL; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virMacAddr macAddr = { 0 }; + char macAddrStr[VIR_MAC_STRING_BUFLEN] = { 0 }; + + VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID); + + iface = g_new0(virDomainInterface, 1); + iface->name = g_strdup(net->data->InstanceID); + + if (hypervDomainDefParseEthernetAdapterMAC(priv, net, &macAddr) < 0) + return -1; + + iface->hwaddr = g_strdup(virMacAddrFormat(&macAddr, macAddrStr)); + + virBufferAsprintf(&query, + "ASSOCIATORS OF {%s} " + "WHERE AssocClass=Msvm_SettingDataComponent " + "ResultClass=Msvm_GuestNetworkAdapterConfiguration", + net->data->Parent);
'query' is unused after this.
It's not.
+ + if (hypervGetWmiClass(Msvm_GuestNetworkAdapterConfiguration, &aConfig) < 0)
This ^^ is actually a macro: #define hypervGetWmiClass(type, class) \ hypervGetWmiClassList(priv, type ## _WmiInfo, &query, (hypervObject **)class)
+ return -1; + + if (aConfig) { + size_t nAddr = aConfig->data->IPAddresses.count; + size_t i; + + if (aConfig->data->Subnets.count != nAddr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("the number of IP addresses (%1$zu) does not match the number of subnets (%2$d)"), + nAddr, aConfig->data->Subnets.count); + return -1; + } + + iface->addrs = g_new0(virDomainIPAddress, nAddr); + iface->naddrs = nAddr; + for (i = 0; i < nAddr; i++) { + const char *ipAddrStr = ((const char **) aConfig->data->IPAddresses.data)[i]; + const char *subnetAddrStr = ((const char **) aConfig->data->Subnets.data)[i]; + virDomainIPAddressPtr ip = &iface->addrs[i]; + int family; + int prefix; + + VIR_DEBUG("ipAddrStr='%s' subnetAddrStr='%s'", + ipAddrStr, subnetAddrStr); + + ip->addr = g_strdup(ipAddrStr); + family = virSocketAddrNumericFamily(ipAddrStr); + if (family == AF_INET6) { + ip->type = VIR_IP_ADDR_TYPE_IPV6; + } else if (family == AF_INET) { + ip->type = VIR_IP_ADDR_TYPE_IPV4; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown IP address family of '%1$s'"), + ipAddrStr); + return -1; + } + + prefix = virSocketAddrSubnetToPrefix(subnetAddrStr); + if (prefix < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected subnet mask '%1$s'"), + subnetAddrStr); + return -1; + } + ip->prefix = prefix; + } + } + + *oneIfaceRet = g_steal_pointer(&iface); + return 0; +} + + +static ssize_t +hypervDomainInterfaceAddressesParseList(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *nets, + virDomainInterfacePtr **ifaces) +{ + Msvm_EthernetPortAllocationSettingData *entry = nets; + size_t nifaces = 0; + + while (entry) { + virDomainInterfacePtr oneIface = NULL; + + if (hypervDomainInterfaceAddressesParseOne(priv, entry, &oneIface) < 0) + return -1;
If this fails ....
+ + if (oneIface) + VIR_APPEND_ELEMENT(*ifaces, nifaces, oneIface);
... after this was partially filled, the caller has no way of figuring out how many entries to free. This function should cleanup after itself internally.
Ah, okay. Will post v2.
+ + entry = entry->next; + } + + return nifaces; +} + +
Michal
On Thu, Feb 12, 2026 at 14:36:37 +0100, Michal Prívozník wrote:
On 2/10/26 18:40, Peter Krempa wrote:
On Fri, Feb 06, 2026 at 14:52:37 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The virDomainInterfaceAddresses() API accepts @source argument, but since this is hyperv, we can't really use _SRC_LEASE (we didn't spawn any dnsmasq there), not _SRC_ARP. The only source that's more or less usable is _SRC_AGENT. Okay, there's no QEMU guest agent running, but hyperv has its own guest agent. In my testing (with Linux guest) I had to install 'hyperv' package and then enable 'hv_kvp_daemon.service'. After that, Msvm_GuestNetworkAdapterConfiguration struct [1] contained guest IP addresses.
There's one caveat though: the interface name (virDomainInterface::name). We don't fetch that one even for hypervDomainGetXMLDesc() case. And there's no <target dev=''/> either nor device alias (v12.0.0-43-g4009126f17). So just put InstanceID there for now, which is this long path, with some UUIDs, e.g.:
Microsoft:5C58E5F2-946E-490F-B81D-6E2A7328640D\C85554E0-2B3B-487C-A557-D230BFF5F9E6\
But hey, at least it's unique.
1: https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/msvm-guestnetworka... Resolves: https://issues.redhat.com/browse/RHEL-145306 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Admittedly, the 'virsh domifaddr' output is misaligned after this, but I'll post a separate patch for that.
Disclaimer: I have absolutely no idea about the hyperv specific bits.
src/hyperv/hyperv_driver.c | 169 ++++++++++++++++++++++++++ src/hyperv/hyperv_wmi_generator.input | 12 ++ 2 files changed, 181 insertions(+)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index fbc76544df..c25cb91c13 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -3654,6 +3654,174 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, }
+static int +hypervDomainInterfaceAddressesParseOne(hypervPrivate *priv, + Msvm_EthernetPortAllocationSettingData *net, + virDomainInterfacePtr *oneIfaceRet) +{ + g_autoptr(virDomainInterface) iface = NULL; + g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL; + g_autoptr(Msvm_GuestNetworkAdapterConfiguration) aConfig = NULL; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virMacAddr macAddr = { 0 }; + char macAddrStr[VIR_MAC_STRING_BUFLEN] = { 0 }; + + VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID); + + iface = g_new0(virDomainInterface, 1); + iface->name = g_strdup(net->data->InstanceID); + + if (hypervDomainDefParseEthernetAdapterMAC(priv, net, &macAddr) < 0) + return -1; + + iface->hwaddr = g_strdup(virMacAddrFormat(&macAddr, macAddrStr)); + + virBufferAsprintf(&query, + "ASSOCIATORS OF {%s} " + "WHERE AssocClass=Msvm_SettingDataComponent " + "ResultClass=Msvm_GuestNetworkAdapterConfiguration", + net->data->Parent);
'query' is unused after this.
It's not.
+ + if (hypervGetWmiClass(Msvm_GuestNetworkAdapterConfiguration, &aConfig) < 0)
This ^^ is actually a macro:
#define hypervGetWmiClass(type, class) \ hypervGetWmiClassList(priv, type ## _WmiInfo, &query, (hypervObject **)class)
Ewwww.
+ return -1;
[...]
+ while (entry) { + virDomainInterfacePtr oneIface = NULL; + + if (hypervDomainInterfaceAddressesParseOne(priv, entry, &oneIface) < 0) + return -1;
If this fails ....
+ + if (oneIface) + VIR_APPEND_ELEMENT(*ifaces, nifaces, oneIface);
... after this was partially filled, the caller has no way of figuring out how many entries to free. This function should cleanup after itself internally.
Ah, okay. Will post v2.
No need to; just fix it and use Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (4)
-
Jonathon Jongsma -
Michal Privoznik -
Michal Prívozník
-
Peter Krempa