Re: [PATCH v3 0/5] qga: Implement shutdown/network-get-interfaces on Solaris
On Tue, 26 Apr 2022 14:55:21 -0500 Andrew Deason wrote: > This implements the guest agent commands guest-network-get-interfaces and > guest-shutdown on Solaris. The implementation for these on Solaris is very > similar as on Linux, since both platforms have a similar getifaddrs() and a > 'shutdown' command. Ping? -- Andrew Deason adea...@sinenomine.net
[PATCH v3 5/5] qga/commands-posix: 'guest-shutdown' for Solaris
On Solaris, instead of the -P, -H, and -r flags, we need to provide
the target init state to the 'shutdown' command: state 5 is poweroff,
0 is halt, and 6 is reboot. We also need to pass -g0 to avoid the
default 60-second delay, and -y to avoid a confirmation prompt.
Implement this logic under an #ifdef CONFIG_SOLARIS, so the
'guest-shutdown' command works properly on Solaris.
Signed-off-by: Andrew Deason
Reviewed-by: Marc-André Lureau
---
Changes since v1:
- new in v2
qga/commands-posix.c | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index cc565e0dfd..386c1ccbc6 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -78,43 +78,58 @@ static void ga_wait_child(pid_t pid, int *status, Error
**errp)
g_assert(rpid == pid);
}
void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
{
const char *shutdown_flag;
Error *local_err = NULL;
pid_t pid;
int status;
+#ifdef CONFIG_SOLARIS
+const char *powerdown_flag = "-i5";
+const char *halt_flag = "-i0";
+const char *reboot_flag = "-i6";
+#else
+const char *powerdown_flag = "-P";
+const char *halt_flag = "-H";
+const char *reboot_flag = "-r";
+#endif
+
slog("guest-shutdown called, mode: %s", mode);
if (!has_mode || strcmp(mode, "powerdown") == 0) {
-shutdown_flag = "-P";
+shutdown_flag = powerdown_flag;
} else if (strcmp(mode, "halt") == 0) {
-shutdown_flag = "-H";
+shutdown_flag = halt_flag;
} else if (strcmp(mode, "reboot") == 0) {
-shutdown_flag = "-r";
+shutdown_flag = reboot_flag;
} else {
error_setg(errp,
"mode is invalid (valid values are: halt|powerdown|reboot");
return;
}
pid = fork();
if (pid == 0) {
/* child, start the shutdown */
setsid();
reopen_fd_to_null(0);
reopen_fd_to_null(1);
reopen_fd_to_null(2);
+#ifdef CONFIG_SOLARIS
+execl("/sbin/shutdown", "shutdown", shutdown_flag, "-g0", "-y",
+ "hypervisor initiated shutdown", (char *)NULL);
+#else
execl("/sbin/shutdown", "shutdown", "-h", shutdown_flag, "+0",
"hypervisor initiated shutdown", (char *)NULL);
+#endif
_exit(EXIT_FAILURE);
} else if (pid < 0) {
error_setg_errno(errp, errno, "failed to create child process");
return;
}
ga_wait_child(pid, , _err);
if (local_err) {
error_propagate(errp, local_err);
return;
--
2.11.0
[PATCH v3 2/5] qga/commands-posix: Fix iface hw address detection
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
Reviewed-by: Michal Privoznik
---
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 75ac19800f..dbfbb14152 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2850,48 +2850,57 @@ GuestNetworkInterfaceList
*qmp_guest_network_get_interfaces(Error **errp)
info = guest_find_interface(head, ifa->ifa_name);
if (!info) {
info = g_malloc0(sizeof(*info));
info->name = g_strdup(ifa->ifa_name);
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) {
error_setg_errno(errp, errno, "failed to create socket");
goto error;
}
memset(, 0, sizeof(ifr));
pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
if (ioctl(sock, SIOCGIFHWADDR, ) == -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 *) _hwaddr.sa_data;
+} else {
+mac_addr = (unsigned char *) _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 &&
ifa->ifa_addr->sa_family == AF_INET) {
/* interface with IPv4 address */
p = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr;
if (!inet_ntop(AF_INET, p, addr4, sizeof(addr4))) {
error_setg_errno(errp, errno, "inet_ntop failed");
goto error;
}
--
2.11.0
[PATCH v3 4/5] qga/commands-posix: Log all net stats failures
guest_get_network_stats can silently fail in a couple of ways. Add
debug messages to these cases, so we're never completely silent on
failure.
Signed-off-by: Andrew Deason
Reviewed-by: Marc-André Lureau
---
Changes since v1:
- new in v2
qga/commands-posix.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index b91fdba2c1..cc565e0dfd 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2764,20 +2764,22 @@ 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) {
+g_debug("failed to open network stats %s: %s", devinfo,
+g_strerror(errno));
return -1;
}
name_len = strlen(name);
while (getline(, , fp) != -1) {
long long dummy;
long long rx_bytes;
long long rx_packets;
long long rx_errs;
long long rx_dropped;
long long tx_bytes;
@@ -2812,21 +2814,23 @@ 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 */
+#else /* !CONFIG_LINUX */
+g_debug("Network stats reporting available only for Linux");
+#endif /* !CONFIG_LINUX */
return -1;
}
/*
* Build information about guest interfaces
*/
GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
{
GuestNetworkInterfaceList *head = NULL, **tail =
struct ifaddrs *ifap, *ifa;
--
2.11.0
[PATCH v3 3/5] qga/commands-posix: Fix listing ifaces for Solaris
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
Reviewed-by: Michal Privoznik
---
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 dbfbb14152..b91fdba2c1 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2756,20 +2756,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);
@@ -2811,20 +2812,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 */
return -1;
}
/*
* Build information about guest interfaces
*/
GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
{
GuestNetworkInterfaceList *head = NULL, **tail =
struct ifaddrs *ifap, *ifa;
@@ -2876,22 +2878,25 @@ GuestNetworkInterfaceList
*qmp_guest_network_get_interfaces(Error **errp)
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 *) _addr.sa_data;
+#else
mac_addr = (unsigned char *) _hwaddr.sa_data;
-
+#endif
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);
}
--
2.11.0
[PATCH v3 1/5] qga/commands-posix: Use getifaddrs when available
Currently, commands-posix.c assumes that getifaddrs() is only
available on Linux, and so the related guest agent command
guest-network-get-interfaces is only implemented for #ifdef __linux__.
This function does exist on other platforms, though, such as Solaris.
So, add a meson check for getifaddrs(), and move the code for
guest-network-get-interfaces to be built whenever getifaddrs() is
available.
The implementation for guest-network-get-interfaces still has some
Linux-specific code, which is not fixed in this commit. This commit
moves the relevant big chunks of code around without changing them, so
a future commit can change the code in place.
Signed-off-by: Andrew Deason
Reviewed-by: Michal Privoznik
---
meson.build | 1 +
qga/commands-posix.c | 474 ++-
2 files changed, 246 insertions(+), 229 deletions(-)
diff --git a/meson.build b/meson.build
index d083c6b7bf..860f593e9f 100644
--- a/meson.build
+++ b/meson.build
@@ -1631,20 +1631,21 @@ config_host_data.set('CONFIG_VALLOC',
cc.has_function('valloc'))
config_host_data.set('CONFIG_MEMALIGN', cc.has_function('memalign'))
config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
'#include '))
config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
cc.has_function('pthread_fchdir_np'))
config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
cc.has_function('unshare'))
config_host_data.set('CONFIG_SYNCFS', cc.has_function('syncfs'))
config_host_data.set('CONFIG_SYNC_FILE_RANGE',
cc.has_function('sync_file_range'))
config_host_data.set('CONFIG_TIMERFD', cc.has_function('timerfd_create'))
config_host_data.set('HAVE_COPY_FILE_RANGE',
cc.has_function('copy_file_range'))
+config_host_data.set('HAVE_GETIFADDRS', cc.has_function('getifaddrs'))
config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies:
util))
config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix:
'#include '))
if rdma.found()
config_host_data.set('HAVE_IBV_ADVISE_MR',
cc.has_function('ibv_advise_mr',
args: config_host['RDMA_LIBS'].split(),
prefix: '#include
'))
endif
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 77f4672ca2..75ac19800f 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -27,38 +27,45 @@
#include "qemu/cutils.h"
#include "commands-common.h"
#ifdef HAVE_UTMPX
#include
#endif
#if defined(__linux__)
#include
#include
-#include
-#include
-#include
-#include
#include
#ifdef CONFIG_LIBUDEV
#include
#endif
#ifdef FIFREEZE
#define CONFIG_FSFREEZE
#endif
#ifdef FITRIM
#define CONFIG_FSTRIM
#endif
#endif
+#ifdef HAVE_GETIFADDRS
+#include
+#include
+#include
+#include
+#include
+#ifdef CONFIG_SOLARIS
+#include
+#endif
+#endif
+
static void ga_wait_child(pid_t pid, int *status, Error **errp)
{
pid_t rpid;
*status = 0;
do {
rpid = waitpid(pid, status, 0);
} while (rpid == -1 && errno == EINTR);
@@ -2122,237 +2129,20 @@ void qmp_guest_suspend_disk(Error **errp)
void qmp_guest_suspend_ram(Error **errp)
{
guest_suspend(SUSPEND_MODE_RAM, errp);
}
void qmp_guest_suspend_hybrid(Error **errp)
{
guest_suspend(SUSPEND_MODE_HYBRID, errp);
}
-static GuestNetworkInterface *
-guest_find_interface(GuestNetworkInterfaceList *head,
- const char *name)
-{
-for (; head; head = head->next) {
-if (strcmp(head->value->name, name) == 0) {
-return head->value;
-}
-}
-
-return NULL;
-}
-
-static int guest_get_network_stats(const char *name,
- GuestNetworkInterfaceStat *stats)
-{
-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);
-while (getline(, , fp) != -1) {
-long long dummy;
-long long rx_bytes;
-long long rx_packets;
-long long rx_errs;
-long long rx_dropped;
-long long tx_bytes;
-long long tx_packets;
-long long tx_errs;
-long long tx_dropped;
-char *trim_line;
-trim_line = g_strchug(line);
-if (trim_line[0] == '\0') {
-continue;
-}
-colon = strchr(trim_line, ':');
-if (!colon) {
-continue;
-}
-if (colon - name_len == trim_line &&
- strncmp(trim_line, name, name_len) == 0) {
-if (sscanf(colon + 1,
-"%lld %l
[PATCH v3 0/5] qga: Implement shutdown/network-get-interfaces on Solaris
This implements the guest agent commands guest-network-get-interfaces and guest-shutdown on Solaris. The implementation for these on Solaris is very similar as on Linux, since both platforms have a similar getifaddrs() and a 'shutdown' command. Changes since v2: - No changes, just updating to newer master Changes since v1: - Add debug messages for failing to get network iface stats - Add implementation for 'guest-shutdown' Andrew Deason (5): qga/commands-posix: Use getifaddrs when available qga/commands-posix: Fix iface hw address detection qga/commands-posix: Fix listing ifaces for Solaris qga/commands-posix: Log all net stats failures qga/commands-posix: 'guest-shutdown' for Solaris meson.build | 1 + qga/commands-posix.c | 513 --- 2 files changed, 282 insertions(+), 232 deletions(-) -- 2.11.0
[PATCH v2 2/5] qga/commands-posix: Fix iface hw address detection
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
Reviewed-by: Michal Privoznik
---
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 e0feb5ffb5..bd0d67f674 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2875,48 +2875,57 @@ GuestNetworkInterfaceList
*qmp_guest_network_get_interfaces(Error **errp)
info = guest_find_interface(head, ifa->ifa_name);
if (!info) {
info = g_malloc0(sizeof(*info));
info->name = g_strdup(ifa->ifa_name);
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) {
error_setg_errno(errp, errno, "failed to create socket");
goto error;
}
memset(, 0, sizeof(ifr));
pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
if (ioctl(sock, SIOCGIFHWADDR, ) == -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 *) _hwaddr.sa_data;
+} else {
+mac_addr = (unsigned char *) _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 &&
ifa->ifa_addr->sa_family == AF_INET) {
/* interface with IPv4 address */
p = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr;
if (!inet_ntop(AF_INET, p, addr4, sizeof(addr4))) {
error_setg_errno(errp, errno, "inet_ntop failed");
goto error;
}
--
2.11.0
[PATCH v2 3/5] qga/commands-posix: Fix listing ifaces for Solaris
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
Reviewed-by: Michal Privoznik
---
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 */
return -1;
}
/*
* Build information about guest interfaces
*/
GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
{
GuestNetworkInterfaceList *head = NULL, **tail =
struct ifaddrs *ifap, *ifa;
@@ -2901,22 +2903,25 @@ GuestNetworkInterfaceList
*qmp_guest_network_get_interfaces(Error **errp)
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 *) _addr.sa_data;
+#else
mac_addr = (unsigned char *) _hwaddr.sa_data;
-
+#endif
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);
}
--
2.11.0
[PATCH v2 5/5] qga/commands-posix: 'guest-shutdown' for Solaris
On Solaris, instead of the -P, -H, and -r flags, we need to provide
the target init state to the 'shutdown' command: state 5 is poweroff,
0 is halt, and 6 is reboot. We also need to pass -g0 to avoid the
default 60-second delay, and -y to avoid a confirmation prompt.
Implement this logic under an #ifdef CONFIG_SOLARIS, so the
'guest-shutdown' command works properly on Solaris.
Signed-off-by: Andrew Deason
---
Changes since v1:
- new in v2
qga/commands-posix.c | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 97e001e998..8c30a9e575 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -88,43 +88,58 @@ static void ga_wait_child(pid_t pid, int *status, Error
**errp)
g_assert(rpid == pid);
}
void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
{
const char *shutdown_flag;
Error *local_err = NULL;
pid_t pid;
int status;
+#ifdef CONFIG_SOLARIS
+const char *powerdown_flag = "-i5";
+const char *halt_flag = "-i0";
+const char *reboot_flag = "-i6";
+#else
+const char *powerdown_flag = "-P";
+const char *halt_flag = "-H";
+const char *reboot_flag = "-r";
+#endif
+
slog("guest-shutdown called, mode: %s", mode);
if (!has_mode || strcmp(mode, "powerdown") == 0) {
-shutdown_flag = "-P";
+shutdown_flag = powerdown_flag;
} else if (strcmp(mode, "halt") == 0) {
-shutdown_flag = "-H";
+shutdown_flag = halt_flag;
} else if (strcmp(mode, "reboot") == 0) {
-shutdown_flag = "-r";
+shutdown_flag = reboot_flag;
} else {
error_setg(errp,
"mode is invalid (valid values are: halt|powerdown|reboot");
return;
}
pid = fork();
if (pid == 0) {
/* child, start the shutdown */
setsid();
reopen_fd_to_null(0);
reopen_fd_to_null(1);
reopen_fd_to_null(2);
+#ifdef CONFIG_SOLARIS
+execle("/sbin/shutdown", "shutdown", shutdown_flag, "-g0", "-y",
+ "hypervisor initiated shutdown", (char *)NULL, environ);
+#else
execle("/sbin/shutdown", "shutdown", "-h", shutdown_flag, "+0",
"hypervisor initiated shutdown", (char *)NULL, environ);
+#endif
_exit(EXIT_FAILURE);
} else if (pid < 0) {
error_setg_errno(errp, errno, "failed to create child process");
return;
}
ga_wait_child(pid, , _err);
if (local_err) {
error_propagate(errp, local_err);
return;
--
2.11.0
[PATCH v2 0/5] qga: Implement shutdown/network-get-interfaces on Solaris
This implements the guest agent commands guest-network-get-interfaces and guest-shutdown on Solaris. The implementation for these on Solaris is very similar as on Linux, since both platforms have a similar getifaddrs() and a 'shutdown' command. Changes since v1: - Add debug messages for failing to get network iface stats - Add implementation for 'guest-shutdown' Andrew Deason (5): qga/commands-posix: Use getifaddrs when available qga/commands-posix: Fix iface hw address detection qga/commands-posix: Fix listing ifaces for Solaris qga/commands-posix: Log all net stats failures qga/commands-posix: 'guest-shutdown' for Solaris meson.build | 1 + qga/commands-posix.c | 513 --- 2 files changed, 282 insertions(+), 232 deletions(-) -- 2.11.0
[PATCH v2 4/5] qga/commands-posix: Log all net stats failures
guest_get_network_stats can silently fail in a couple of ways. Add
debug messages to these cases, so we're never completely silent on
failure.
Signed-off-by: Andrew Deason
---
Changes since v1:
- new in v2
qga/commands-posix.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index c0b00fc488..97e001e998 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2789,20 +2789,22 @@ 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) {
+g_debug("failed to open network stats %s: %s", devinfo,
+g_strerror(errno));
return -1;
}
name_len = strlen(name);
while (getline(, , fp) != -1) {
long long dummy;
long long rx_bytes;
long long rx_packets;
long long rx_errs;
long long rx_dropped;
long long tx_bytes;
@@ -2837,21 +2839,23 @@ 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 */
+#else /* !CONFIG_LINUX */
+g_debug("Network stats reporting available only for Linux");
+#endif /* !CONFIG_LINUX */
return -1;
}
/*
* Build information about guest interfaces
*/
GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
{
GuestNetworkInterfaceList *head = NULL, **tail =
struct ifaddrs *ifap, *ifa;
--
2.11.0
[PATCH v2 1/5] qga/commands-posix: Use getifaddrs when available
Currently, commands-posix.c assumes that getifaddrs() is only
available on Linux, and so the related guest agent command
guest-network-get-interfaces is only implemented for #ifdef __linux__.
This function does exist on other platforms, though, such as Solaris.
So, add a meson check for getifaddrs(), and move the code for
guest-network-get-interfaces to be built whenever getifaddrs() is
available.
The implementation for guest-network-get-interfaces still has some
Linux-specific code, which is not fixed in this commit. This commit
moves the relevant big chunks of code around without changing them, so
a future commit can change the code in place.
Signed-off-by: Andrew Deason
Reviewed-by: Michal Privoznik
---
meson.build | 1 +
qga/commands-posix.c | 474 ++-
2 files changed, 246 insertions(+), 229 deletions(-)
diff --git a/meson.build b/meson.build
index 861de93c4f..1c033bcc58 100644
--- a/meson.build
+++ b/meson.build
@@ -1633,20 +1633,21 @@ config_host_data.set('CONFIG_MEMALIGN',
cc.has_function('memalign'))
config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
'#include '))
config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
cc.has_function('pthread_fchdir_np'))
config_host_data.set('CONFIG_SEM_TIMEDWAIT', cc.has_function('sem_timedwait',
dependencies: threads))
config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
cc.has_function('unshare'))
config_host_data.set('CONFIG_SYNCFS', cc.has_function('syncfs'))
config_host_data.set('CONFIG_SYNC_FILE_RANGE',
cc.has_function('sync_file_range'))
config_host_data.set('CONFIG_TIMERFD', cc.has_function('timerfd_create'))
config_host_data.set('HAVE_COPY_FILE_RANGE',
cc.has_function('copy_file_range'))
+config_host_data.set('HAVE_GETIFADDRS', cc.has_function('getifaddrs'))
config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies:
util))
config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix:
'#include '))
if rdma.found()
config_host_data.set('HAVE_IBV_ADVISE_MR',
cc.has_function('ibv_advise_mr',
args: config_host['RDMA_LIBS'].split(),
prefix: '#include
'))
endif
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 75dbaab68e..e0feb5ffb5 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -37,38 +37,45 @@
#include
#define environ (*_NSGetEnviron())
#else
extern char **environ;
#endif
#endif
#if defined(__linux__)
#include
#include
-#include
-#include
-#include
-#include
#include
#ifdef CONFIG_LIBUDEV
#include
#endif
#ifdef FIFREEZE
#define CONFIG_FSFREEZE
#endif
#ifdef FITRIM
#define CONFIG_FSTRIM
#endif
#endif
+#ifdef HAVE_GETIFADDRS
+#include
+#include
+#include
+#include
+#include
+#ifdef CONFIG_SOLARIS
+#include
+#endif
+#endif
+
static void ga_wait_child(pid_t pid, int *status, Error **errp)
{
pid_t rpid;
*status = 0;
do {
rpid = waitpid(pid, status, 0);
} while (rpid == -1 && errno == EINTR);
@@ -2147,237 +2154,20 @@ void qmp_guest_suspend_disk(Error **errp)
void qmp_guest_suspend_ram(Error **errp)
{
guest_suspend(SUSPEND_MODE_RAM, errp);
}
void qmp_guest_suspend_hybrid(Error **errp)
{
guest_suspend(SUSPEND_MODE_HYBRID, errp);
}
-static GuestNetworkInterface *
-guest_find_interface(GuestNetworkInterfaceList *head,
- const char *name)
-{
-for (; head; head = head->next) {
-if (strcmp(head->value->name, name) == 0) {
-return head->value;
-}
-}
-
-return NULL;
-}
-
-static int guest_get_network_stats(const char *name,
- GuestNetworkInterfaceStat *stats)
-{
-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);
-while (getline(, , fp) != -1) {
-long long dummy;
-long long rx_bytes;
-long long rx_packets;
-long long rx_errs;
-long long rx_dropped;
-long long tx_bytes;
-long long tx_packets;
-long long tx_errs;
-long long tx_dropped;
-char *trim_line;
-trim_line = g_strchug(line);
-if (trim_line[0] == '\0') {
-continue;
-}
-colon = strchr(trim_line, ':');
-if (!colon) {
-continue;
-}
-if (colon - name_len == trim_line &&
- strncmp(trim_line, name, name_len) == 0) {
-if (sscanf(colon + 1,
-
Re: [PATCH 3/3] qga/commands-posix: Fix listing ifaces for Solaris
On Mon, 21 Mar 2022 10:14:57 +0100
Michal Prívozník 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
> > ---
> > 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
Re: [PATCH] softmmu/physmem: Use qemu_madvise
On Tue, 22 Mar 2022 17:58:19 +
"Dr. David Alan Gilbert" wrote:
> * Andrew Deason (adea...@sinenomine.net) wrote:
> > On Tue, 22 Mar 2022 16:53:05 +
> > "Dr. David Alan Gilbert" wrote:
> >
> > > * Andrew Deason (adea...@sinenomine.net) wrote:
> > > > On Wed, 16 Mar 2022 10:41:41 +0100
> > > > David Hildenbrand wrote:
> > > >
> > > > > On 16.03.22 10:37, Dr. David Alan Gilbert wrote:
> > > > > > * Peter Maydell (peter.mayd...@linaro.org) wrote:
> > > > > >> On Wed, 16 Mar 2022 at 07:53, David Hildenbrand
> > > > > >> wrote:
> > > > > >>>
> > > > > >>> On 16.03.22 05:04, Andrew Deason wrote:
> > > > > >>>> We have a thin wrapper around madvise, called qemu_madvise, which
> > > > > >>>> provides consistent behavior for the !CONFIG_MADVISE case, and
> > > > > >>>> works
> > > > > >>>> around some platform-specific quirks (some platforms only provide
> > > > > >>>> posix_madvise, and some don't offer all 'advise' types). This
> > > > > >>>> specific
> > > > > >>>> caller of madvise has never used it, tracing back to its original
> > > > > >>>> introduction in commit e0b266f01dd2 ("migration_completion: Take
> > > > > >>>> current state").
> > > > > >>>>
> > > > > >>>> Call qemu_madvise here, to follow the same logic as all of our
> > > > > >>>> other
> > > > > >>>> madvise callers. This slightly changes the behavior for
> > > > > >>>> !CONFIG_MADVISE (EINVAL instead of ENOSYS, and a slightly
> > > > > >>>> different
> > > > > >>>> error message), but this is now more consistent with other
> > > > > >>>> callers
> > > > > >>>> that use qemu_madvise.
> > > > > >>>>
> > > > > >>>> Signed-off-by: Andrew Deason
> > > > > >>>> ---
> > > > > >>>> Looking at the history of commits that touch this madvise()
> > > > > >>>> call, it
> > > > > >>>> doesn't _look_ like there's any reason to be directly calling
> > > > > >>>> madvise vs
> > > > > >>>> qemu_advise (I don't see anything mentioned), but I'm not sure.
> > > > > >>>>
> > > > > >>>> softmmu/physmem.c | 12 ++--
> > > > > >>>> 1 file changed, 2 insertions(+), 10 deletions(-)
> > > > > >>>>
> > > > > >>>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> > > > > >>>> index 43ae70fbe2..900c692b5e 100644
> > > > > >>>> --- a/softmmu/physmem.c
> > > > > >>>> +++ b/softmmu/physmem.c
> > > > > >>>> @@ -3584,40 +3584,32 @@ int ram_block_discard_range(RAMBlock
> > > > > >>>> *rb, uint64_t start, size_t length)
> > > > > >>>> rb->idstr, start, length, ret);
> > > > > >>>> goto err;
> > > > > >>>> #endif
> > > > > >>>> }
> > > > > >>>> if (need_madvise) {
> > > > > >>>> /* For normal RAM this causes it to be unmapped,
> > > > > >>>> * for shared memory it causes the local mapping to
> > > > > >>>> disappear
> > > > > >>>> * and to fall back on the file contents (which we
> > > > > >>>> just
> > > > > >>>> * fallocate'd away).
> > > > > >>>> */
> > > > > >>>> -#if defined(CONFIG_MADVISE)
> > > > > >>>> if (qemu_ram_is_shared(rb) && rb->fd < 0) {
> > > > > >>>> -ret = madvise(host_startaddr, length,
> > > > > >>>> QEMU_MADV_REMOVE);
> > > > > >>>> +ret = qemu_madvise(host_startaddr, length,
> > > > > >>>
Re: [PATCH] softmmu/physmem: Use qemu_madvise
On Tue, 22 Mar 2022 16:53:05 +
"Dr. David Alan Gilbert" wrote:
> * Andrew Deason (adea...@sinenomine.net) wrote:
> > On Wed, 16 Mar 2022 10:41:41 +0100
> > David Hildenbrand wrote:
> >
> > > On 16.03.22 10:37, Dr. David Alan Gilbert wrote:
> > > > * Peter Maydell (peter.mayd...@linaro.org) wrote:
> > > >> On Wed, 16 Mar 2022 at 07:53, David Hildenbrand
> > > >> wrote:
> > > >>>
> > > >>> On 16.03.22 05:04, Andrew Deason wrote:
> > > >>>> We have a thin wrapper around madvise, called qemu_madvise, which
> > > >>>> provides consistent behavior for the !CONFIG_MADVISE case, and works
> > > >>>> around some platform-specific quirks (some platforms only provide
> > > >>>> posix_madvise, and some don't offer all 'advise' types). This
> > > >>>> specific
> > > >>>> caller of madvise has never used it, tracing back to its original
> > > >>>> introduction in commit e0b266f01dd2 ("migration_completion: Take
> > > >>>> current state").
> > > >>>>
> > > >>>> Call qemu_madvise here, to follow the same logic as all of our other
> > > >>>> madvise callers. This slightly changes the behavior for
> > > >>>> !CONFIG_MADVISE (EINVAL instead of ENOSYS, and a slightly different
> > > >>>> error message), but this is now more consistent with other callers
> > > >>>> that use qemu_madvise.
> > > >>>>
> > > >>>> Signed-off-by: Andrew Deason
> > > >>>> ---
> > > >>>> Looking at the history of commits that touch this madvise() call, it
> > > >>>> doesn't _look_ like there's any reason to be directly calling
> > > >>>> madvise vs
> > > >>>> qemu_advise (I don't see anything mentioned), but I'm not sure.
> > > >>>>
> > > >>>> softmmu/physmem.c | 12 ++--
> > > >>>> 1 file changed, 2 insertions(+), 10 deletions(-)
> > > >>>>
> > > >>>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> > > >>>> index 43ae70fbe2..900c692b5e 100644
> > > >>>> --- a/softmmu/physmem.c
> > > >>>> +++ b/softmmu/physmem.c
> > > >>>> @@ -3584,40 +3584,32 @@ int ram_block_discard_range(RAMBlock *rb,
> > > >>>> uint64_t start, size_t length)
> > > >>>> rb->idstr, start, length, ret);
> > > >>>> goto err;
> > > >>>> #endif
> > > >>>> }
> > > >>>> if (need_madvise) {
> > > >>>> /* For normal RAM this causes it to be unmapped,
> > > >>>> * for shared memory it causes the local mapping to
> > > >>>> disappear
> > > >>>> * and to fall back on the file contents (which we just
> > > >>>> * fallocate'd away).
> > > >>>> */
> > > >>>> -#if defined(CONFIG_MADVISE)
> > > >>>> if (qemu_ram_is_shared(rb) && rb->fd < 0) {
> > > >>>> -ret = madvise(host_startaddr, length,
> > > >>>> QEMU_MADV_REMOVE);
> > > >>>> +ret = qemu_madvise(host_startaddr, length,
> > > >>>> QEMU_MADV_REMOVE);
> > > >>>> } else {
> > > >>>> -ret = madvise(host_startaddr, length,
> > > >>>> QEMU_MADV_DONTNEED);
> > > >>>> +ret = qemu_madvise(host_startaddr, length,
> > > >>>> QEMU_MADV_DONTNEED);
> > > >>>
> > > >>> posix_madvise(QEMU_MADV_DONTNEED) has completely different semantics
> > > >>> then madvise() -- it's not a discard that we need here.
> > > >>>
> > > >>> So ram_block_discard_range() would now succeed in environments (BSD?)
> > > >>> where it's supposed to fail.
> > > >>>
> > > >>> So AFAIKs this isn't sane.
> > > >>
> > > >> But CONFIG_MADVISE just means "host has madvise()"; it doesn't imply
> > > >> "thi
Re: [PATCH] softmmu/physmem: Use qemu_madvise
On Wed, 16 Mar 2022 10:41:41 +0100
David Hildenbrand wrote:
> On 16.03.22 10:37, Dr. David Alan Gilbert wrote:
> > * Peter Maydell (peter.mayd...@linaro.org) wrote:
> >> On Wed, 16 Mar 2022 at 07:53, David Hildenbrand wrote:
> >>>
> >>> On 16.03.22 05:04, Andrew Deason wrote:
> >>>> We have a thin wrapper around madvise, called qemu_madvise, which
> >>>> provides consistent behavior for the !CONFIG_MADVISE case, and works
> >>>> around some platform-specific quirks (some platforms only provide
> >>>> posix_madvise, and some don't offer all 'advise' types). This specific
> >>>> caller of madvise has never used it, tracing back to its original
> >>>> introduction in commit e0b266f01dd2 ("migration_completion: Take
> >>>> current state").
> >>>>
> >>>> Call qemu_madvise here, to follow the same logic as all of our other
> >>>> madvise callers. This slightly changes the behavior for
> >>>> !CONFIG_MADVISE (EINVAL instead of ENOSYS, and a slightly different
> >>>> error message), but this is now more consistent with other callers
> >>>> that use qemu_madvise.
> >>>>
> >>>> Signed-off-by: Andrew Deason
> >>>> ---
> >>>> Looking at the history of commits that touch this madvise() call, it
> >>>> doesn't _look_ like there's any reason to be directly calling madvise vs
> >>>> qemu_advise (I don't see anything mentioned), but I'm not sure.
> >>>>
> >>>> softmmu/physmem.c | 12 ++--
> >>>> 1 file changed, 2 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> >>>> index 43ae70fbe2..900c692b5e 100644
> >>>> --- a/softmmu/physmem.c
> >>>> +++ b/softmmu/physmem.c
> >>>> @@ -3584,40 +3584,32 @@ int ram_block_discard_range(RAMBlock *rb,
> >>>> uint64_t start, size_t length)
> >>>> rb->idstr, start, length, ret);
> >>>> goto err;
> >>>> #endif
> >>>> }
> >>>> if (need_madvise) {
> >>>> /* For normal RAM this causes it to be unmapped,
> >>>> * for shared memory it causes the local mapping to
> >>>> disappear
> >>>> * and to fall back on the file contents (which we just
> >>>> * fallocate'd away).
> >>>> */
> >>>> -#if defined(CONFIG_MADVISE)
> >>>> if (qemu_ram_is_shared(rb) && rb->fd < 0) {
> >>>> -ret = madvise(host_startaddr, length, QEMU_MADV_REMOVE);
> >>>> +ret = qemu_madvise(host_startaddr, length,
> >>>> QEMU_MADV_REMOVE);
> >>>> } else {
> >>>> -ret = madvise(host_startaddr, length,
> >>>> QEMU_MADV_DONTNEED);
> >>>> +ret = qemu_madvise(host_startaddr, length,
> >>>> QEMU_MADV_DONTNEED);
> >>>
> >>> posix_madvise(QEMU_MADV_DONTNEED) has completely different semantics
> >>> then madvise() -- it's not a discard that we need here.
> >>>
> >>> So ram_block_discard_range() would now succeed in environments (BSD?)
> >>> where it's supposed to fail.
> >>>
> >>> So AFAIKs this isn't sane.
> >>
> >> But CONFIG_MADVISE just means "host has madvise()"; it doesn't imply
> >> "this is a Linux madvise() with MADV_DONTNEED". Solaris madvise()
> >> doesn't seem to have MADV_DONTNEED at all; a quick look at the
> >> FreeBSD manpage suggests its madvise MADV_DONTNEED is identical
> >> to its posix_madvise MADV_DONTNEED.
> >>
> >> If we need "specifically Linux MADV_DONTNEED semantics" maybe we
> >> should define a QEMU_MADV_LINUX_DONTNEED which either (a) does the
> >> right thing or (b) fails, and use qemu_madvise() regardless.
> >>
> >> Certainly the current code is pretty fragile to being changed by
> >> people who don't understand the undocumented subtlety behind
> >> the use of a direct madvise() call here.
> >
> > Yeh and I'm not sure I can remembe rall the subtleties; there's a big
> > hairy set of ifdef's in include/qemu/madvise.h that makes
> > sure we always have the def
[PATCH] io: Limit readv/writev to IOV_MAX
The unit tests 'test-io-channel-file' and 'test-io-channel-socket'
currently fail on Solaris, because we try to perform vectored I/O with
a batch of 250 (CHUNK_COUNT) iovs. This exceeds MAX_IOV on Solaris
(only 16, much lower than Linux's 1024), and so results in an EINVAL
for file operations, and EMSGSIZE for sockets.
To fix this, make qio_channel_readv_full and qio_channel_writev_full
only use the first IOV_MAX iovs passed in, and we'll return the number
of bytes actually processed like always.
Signed-off-by: Andrew Deason
---
These aren't the only tests that fail on Solaris right now, but this
seemed easy to fix (assuming this is correct), so I'm just submitting
something while I'm looking at it.
io/channel.c | 4
1 file changed, 4 insertions(+)
diff --git a/io/channel.c b/io/channel.c
index e8b019dc36..cb9d85a30d 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -56,40 +56,44 @@ ssize_t qio_channel_readv_full(QIOChannel *ioc,
{
QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
if ((fds || nfds) &&
!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
error_setg_errno(errp, EINVAL,
"Channel does not support file descriptor passing");
return -1;
}
+niov = MIN(niov, IOV_MAX);
+
return klass->io_readv(ioc, iov, niov, fds, nfds, errp);
}
ssize_t qio_channel_writev_full(QIOChannel *ioc,
const struct iovec *iov,
size_t niov,
int *fds,
size_t nfds,
Error **errp)
{
QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
if ((fds || nfds) &&
!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
error_setg_errno(errp, EINVAL,
"Channel does not support file descriptor passing");
return -1;
}
+niov = MIN(niov, IOV_MAX);
+
return klass->io_writev(ioc, iov, niov, fds, nfds, errp);
}
int qio_channel_readv_all_eof(QIOChannel *ioc,
const struct iovec *iov,
size_t niov,
Error **errp)
{
return qio_channel_readv_full_all_eof(ioc, iov, niov, NULL, NULL, errp);
--
2.11.0
[PATCH 2/3] qga/commands-posix: Fix iface hw address detection
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
---
As implemented, this commit results in a behavior difference between Linux and
Solaris. It's fixable, but I'm not sure which behavior we should have, or if
it's okay that they behave differently:
On Linux, we've always returned the MAC address for loopback as all zeroes. In
this commit, on Solaris we don't return a MAC address at all for loopback
(because trying to get the MAC for lo results in an error).
It doesn't have to be like this; the behavior could be made the same if we
wanted, by either:
- Making it so failing to get the hw address falls back to providing an
all-zeroes address (making Solaris look more like Linux).
- Stop providing a 'fake' all-zeroes hw address on Linux (arguably more
accurate, but harder to detect, and different than past behavior).
Or we could just leave the behavior as it is and have the two behave slightly
differently; we're merely reflecting what the underlying platform does, and I'm
not sure anyone cares about the difference. I assume this applies to any
network interface that doesn't have a hardware address, but loopback is the
only one I've seen on the simple test VMs I've been using so far.
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 e0feb5ffb5..bd0d67f674 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2875,48 +2875,57 @@ GuestNetworkInterfaceList
*qmp_guest_network_get_interfaces(Error **errp)
info = guest_find_interface(head, ifa->ifa_name);
if (!info) {
info = g_malloc0(sizeof(*info));
info->name = g_strdup(ifa->ifa_name);
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) {
error_setg_errno(errp, errno, "failed to create socket");
goto error;
}
memset(, 0, sizeof(ifr));
pstrcpy(ifr.ifr_name, IF_NAMESIZE, info->name);
if (ioctl(sock, SIOCGIFHWADDR, ) == -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 *) _hwaddr.sa_data;
+} else {
+mac_addr = (unsigned char *) _hwaddr.sa_data;
-info->hardware_address =
-g_strdup_printf("%02x:%02x:%02x:%02x:%02x:%02x",
-
[PATCH 1/3] qga/commands-posix: Use getifaddrs when available
Currently, commands-posix.c assumes that getifaddrs() is only
available on Linux, and so the related guest agent command
guest-network-get-interfaces is only implemented for #ifdef __linux__.
This function does exist on other platforms, though, such as Solaris.
So, add a meson check for getifaddrs(), and move the code for
guest-network-get-interfaces to be built whenever getifaddrs() is
available.
The implementation for guest-network-get-interfaces still has some
Linux-specific code, which is not fixed in this commit. This commit
moves the relevant big chunks of code around without changing them, so
a future commit can change the code in place.
Signed-off-by: Andrew Deason
---
meson.build | 1 +
qga/commands-posix.c | 474 ++-
2 files changed, 246 insertions(+), 229 deletions(-)
diff --git a/meson.build b/meson.build
index 282e7c4650..8d7555cb74 100644
--- a/meson.build
+++ b/meson.build
@@ -1633,20 +1633,21 @@ config_host_data.set('CONFIG_MEMALIGN',
cc.has_function('memalign'))
config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
'#include '))
config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
cc.has_function('pthread_fchdir_np'))
config_host_data.set('CONFIG_SEM_TIMEDWAIT', cc.has_function('sem_timedwait',
dependencies: threads))
config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
cc.has_function('unshare'))
config_host_data.set('CONFIG_SYNCFS', cc.has_function('syncfs'))
config_host_data.set('CONFIG_SYNC_FILE_RANGE',
cc.has_function('sync_file_range'))
config_host_data.set('CONFIG_TIMERFD', cc.has_function('timerfd_create'))
config_host_data.set('HAVE_COPY_FILE_RANGE',
cc.has_function('copy_file_range'))
+config_host_data.set('HAVE_GETIFADDRS', cc.has_function('getifaddrs'))
config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies:
util))
config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix:
'#include '))
if rdma.found()
config_host_data.set('HAVE_IBV_ADVISE_MR',
cc.has_function('ibv_advise_mr',
args: config_host['RDMA_LIBS'].split(),
prefix: '#include
'))
endif
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 75dbaab68e..e0feb5ffb5 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -37,38 +37,45 @@
#include
#define environ (*_NSGetEnviron())
#else
extern char **environ;
#endif
#endif
#if defined(__linux__)
#include
#include
-#include
-#include
-#include
-#include
#include
#ifdef CONFIG_LIBUDEV
#include
#endif
#ifdef FIFREEZE
#define CONFIG_FSFREEZE
#endif
#ifdef FITRIM
#define CONFIG_FSTRIM
#endif
#endif
+#ifdef HAVE_GETIFADDRS
+#include
+#include
+#include
+#include
+#include
+#ifdef CONFIG_SOLARIS
+#include
+#endif
+#endif
+
static void ga_wait_child(pid_t pid, int *status, Error **errp)
{
pid_t rpid;
*status = 0;
do {
rpid = waitpid(pid, status, 0);
} while (rpid == -1 && errno == EINTR);
@@ -2147,237 +2154,20 @@ void qmp_guest_suspend_disk(Error **errp)
void qmp_guest_suspend_ram(Error **errp)
{
guest_suspend(SUSPEND_MODE_RAM, errp);
}
void qmp_guest_suspend_hybrid(Error **errp)
{
guest_suspend(SUSPEND_MODE_HYBRID, errp);
}
-static GuestNetworkInterface *
-guest_find_interface(GuestNetworkInterfaceList *head,
- const char *name)
-{
-for (; head; head = head->next) {
-if (strcmp(head->value->name, name) == 0) {
-return head->value;
-}
-}
-
-return NULL;
-}
-
-static int guest_get_network_stats(const char *name,
- GuestNetworkInterfaceStat *stats)
-{
-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);
-while (getline(, , fp) != -1) {
-long long dummy;
-long long rx_bytes;
-long long rx_packets;
-long long rx_errs;
-long long rx_dropped;
-long long tx_bytes;
-long long tx_packets;
-long long tx_errs;
-long long tx_dropped;
-char *trim_line;
-trim_line = g_strchug(line);
-if (trim_line[0] == '\0') {
-continue;
-}
-colon = strchr(trim_line, ':');
-if (!colon) {
-continue;
-}
-if (colon - name_len == trim_line &&
- strncmp(trim_line, name, name_len) == 0) {
-if (sscanf(colon + 1,
-"%lld %lld %lld %lld %l
[PATCH 0/3] qga: Implement guest-network-get-interfaces for Solaris
This implements the guest agent guest-network-get-interfaces command on Solaris. Solaris provides a getifaddrs() that's very similar to the Linux one, so the implementation is mostly the same. Andrew Deason (3): qga/commands-posix: Use getifaddrs when available qga/commands-posix: Fix iface hw address detection qga/commands-posix: Fix listing ifaces for Solaris meson.build | 1 + qga/commands-posix.c | 488 +++ 2 files changed, 260 insertions(+), 229 deletions(-) -- 2.11.0
[PATCH 3/3] qga/commands-posix: Fix listing ifaces for Solaris
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
---
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 */
return -1;
}
/*
* Build information about guest interfaces
*/
GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
{
GuestNetworkInterfaceList *head = NULL, **tail =
struct ifaddrs *ifap, *ifa;
@@ -2901,22 +2903,25 @@ GuestNetworkInterfaceList
*qmp_guest_network_get_interfaces(Error **errp)
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 *) _addr.sa_data;
+#else
mac_addr = (unsigned char *) _hwaddr.sa_data;
-
+#endif
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);
}
--
2.11.0
Re: [PATCH] softmmu/physmem: Use qemu_madvise
On Wed, 16 Mar 2022 10:41:41 +0100
David Hildenbrand wrote:
> On 16.03.22 10:37, Dr. David Alan Gilbert wrote:
> > * Peter Maydell (peter.mayd...@linaro.org) wrote:
> >> On Wed, 16 Mar 2022 at 07:53, David Hildenbrand wrote:
> >>>
> >>> On 16.03.22 05:04, Andrew Deason wrote:
> >>>> We have a thin wrapper around madvise, called qemu_madvise, which
> >>>> provides consistent behavior for the !CONFIG_MADVISE case, and works
> >>>> around some platform-specific quirks (some platforms only provide
> >>>> posix_madvise, and some don't offer all 'advise' types). This specific
> >>>> caller of madvise has never used it, tracing back to its original
> >>>> introduction in commit e0b266f01dd2 ("migration_completion: Take
> >>>> current state").
> >>>>
> >>>> Call qemu_madvise here, to follow the same logic as all of our other
> >>>> madvise callers. This slightly changes the behavior for
> >>>> !CONFIG_MADVISE (EINVAL instead of ENOSYS, and a slightly different
> >>>> error message), but this is now more consistent with other callers
> >>>> that use qemu_madvise.
> >>>>
> >>>> Signed-off-by: Andrew Deason
> >>>> ---
> >>>> Looking at the history of commits that touch this madvise() call, it
> >>>> doesn't _look_ like there's any reason to be directly calling madvise vs
> >>>> qemu_advise (I don't see anything mentioned), but I'm not sure.
> >>>>
> >>>> softmmu/physmem.c | 12 ++--
> >>>> 1 file changed, 2 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> >>>> index 43ae70fbe2..900c692b5e 100644
> >>>> --- a/softmmu/physmem.c
> >>>> +++ b/softmmu/physmem.c
> >>>> @@ -3584,40 +3584,32 @@ int ram_block_discard_range(RAMBlock *rb,
> >>>> uint64_t start, size_t length)
> >>>> rb->idstr, start, length, ret);
> >>>> goto err;
> >>>> #endif
> >>>> }
> >>>> if (need_madvise) {
> >>>> /* For normal RAM this causes it to be unmapped,
> >>>> * for shared memory it causes the local mapping to
> >>>> disappear
> >>>> * and to fall back on the file contents (which we just
> >>>> * fallocate'd away).
> >>>> */
> >>>> -#if defined(CONFIG_MADVISE)
> >>>> if (qemu_ram_is_shared(rb) && rb->fd < 0) {
> >>>> -ret = madvise(host_startaddr, length, QEMU_MADV_REMOVE);
> >>>> +ret = qemu_madvise(host_startaddr, length,
> >>>> QEMU_MADV_REMOVE);
> >>>> } else {
> >>>> -ret = madvise(host_startaddr, length,
> >>>> QEMU_MADV_DONTNEED);
> >>>> +ret = qemu_madvise(host_startaddr, length,
> >>>> QEMU_MADV_DONTNEED);
> >>>
> >>> posix_madvise(QEMU_MADV_DONTNEED) has completely different semantics
> >>> then madvise() -- it's not a discard that we need here.
> >>>
> >>> So ram_block_discard_range() would now succeed in environments (BSD?)
> >>> where it's supposed to fail.
> >>>
> >>> So AFAIKs this isn't sane.
> >>
> >> But CONFIG_MADVISE just means "host has madvise()"; it doesn't imply
> >> "this is a Linux madvise() with MADV_DONTNEED". Solaris madvise()
> >> doesn't seem to have MADV_DONTNEED at all; a quick look at the
> >> FreeBSD manpage suggests its madvise MADV_DONTNEED is identical
> >> to its posix_madvise MADV_DONTNEED.
> >>
> >> If we need "specifically Linux MADV_DONTNEED semantics" maybe we
> >> should define a QEMU_MADV_LINUX_DONTNEED which either (a) does the
> >> right thing or (b) fails, and use qemu_madvise() regardless.
> >>
> >> Certainly the current code is pretty fragile to being changed by
> >> people who don't understand the undocumented subtlety behind
> >> the use of a direct madvise() call here.
> >
> > Yeh and I'm not sure I can remembe rall the subtleties; there's a big
> > hairy set of ifdef's in include/qemu/madvise.h that makes
> > sure we always have the de
[PATCH] softmmu/physmem: Use qemu_madvise
We have a thin wrapper around madvise, called qemu_madvise, which
provides consistent behavior for the !CONFIG_MADVISE case, and works
around some platform-specific quirks (some platforms only provide
posix_madvise, and some don't offer all 'advise' types). This specific
caller of madvise has never used it, tracing back to its original
introduction in commit e0b266f01dd2 ("migration_completion: Take
current state").
Call qemu_madvise here, to follow the same logic as all of our other
madvise callers. This slightly changes the behavior for
!CONFIG_MADVISE (EINVAL instead of ENOSYS, and a slightly different
error message), but this is now more consistent with other callers
that use qemu_madvise.
Signed-off-by: Andrew Deason
---
Looking at the history of commits that touch this madvise() call, it
doesn't _look_ like there's any reason to be directly calling madvise vs
qemu_advise (I don't see anything mentioned), but I'm not sure.
softmmu/physmem.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 43ae70fbe2..900c692b5e 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3584,40 +3584,32 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t
start, size_t length)
rb->idstr, start, length, ret);
goto err;
#endif
}
if (need_madvise) {
/* For normal RAM this causes it to be unmapped,
* for shared memory it causes the local mapping to disappear
* and to fall back on the file contents (which we just
* fallocate'd away).
*/
-#if defined(CONFIG_MADVISE)
if (qemu_ram_is_shared(rb) && rb->fd < 0) {
-ret = madvise(host_startaddr, length, QEMU_MADV_REMOVE);
+ret = qemu_madvise(host_startaddr, length, QEMU_MADV_REMOVE);
} else {
-ret = madvise(host_startaddr, length, QEMU_MADV_DONTNEED);
+ret = qemu_madvise(host_startaddr, length, QEMU_MADV_DONTNEED);
}
if (ret) {
ret = -errno;
error_report("ram_block_discard_range: Failed to discard range
"
"%s:%" PRIx64 " +%zx (%d)",
rb->idstr, start, length, ret);
goto err;
}
-#else
-ret = -ENOSYS;
-error_report("ram_block_discard_range: MADVISE not available"
- "%s:%" PRIx64 " +%zx (%d)",
- rb->idstr, start, length, ret);
-goto err;
-#endif
}
trace_ram_block_discard_range(rb->idstr, host_startaddr, length,
need_madvise, need_fallocate, ret);
} else {
error_report("ram_block_discard_range: Overrun block '%s' (%" PRIu64
"/%zx/" RAM_ADDR_FMT")",
rb->idstr, start, length, rb->max_length);
}
err:
--
2.11.0
[PATCH v3 2/3] hw/i386/acpi-build: Avoid 'sun' identifier
On Solaris, 'sun' is #define'd to 1, which causes errors if a variable
is named 'sun'. Slightly change the name of the var for the Slot User
Number so we can build on Solaris.
Reviewed-by: Ani Sinha
Signed-off-by: Andrew Deason
---
hw/i386/acpi-build.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4ad4d7286c..dcf6ece3d0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -576,32 +576,32 @@ static void build_append_pci_bus_devices(Aml
*parent_scope, PCIBus *bus,
}
Aml *aml_pci_device_dsm(void)
{
Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
Aml *acpi_index = aml_local(0);
Aml *zero = aml_int(0);
Aml *bnum = aml_arg(4);
Aml *func = aml_arg(2);
Aml *rev = aml_arg(1);
-Aml *sun = aml_arg(5);
+Aml *sunum = aml_arg(5);
method = aml_method("PDSM", 6, AML_SERIALIZED);
/*
* PCI Firmware Specification 3.1
* 4.6. _DSM Definitions for PCI
*/
UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
ifctx = aml_if(aml_equal(aml_arg(0), UUID));
{
-aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
+aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sunum),
acpi_index));
ifctx1 = aml_if(aml_equal(func, zero));
{
uint8_t byte_list[1];
ifctx2 = aml_if(aml_equal(rev, aml_int(2)));
{
/*
* advertise function 7 if device has acpi-index
* acpi_index values:
*0: not present (default value)
--
2.11.0
[PATCH v3 3/3] util/osdep: Remove some early cruft
The include for statvfs.h has not been needed since all statvfs calls
were removed in commit 4a1418e07bdc ("Unbreak large mem support by
removing kqemu").
The comment mentioning CONFIG_BSD hasn't made sense since an include
for config-host.h was removed in commit aafd75841001 ("util: Clean up
includes").
Remove this cruft.
Reviewed-by: Peter Maydell
Signed-off-by: Andrew Deason
---
util/osdep.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/util/osdep.c b/util/osdep.c
index 1825399bcf..394804d32e 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -16,27 +16,20 @@
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
#include "qapi/error.h"
-
-/* Needed early for CONFIG_BSD etc. */
-
-#ifdef CONFIG_SOLARIS
-#include
-#endif
-
#include "qemu-common.h"
#include "qemu/cutils.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
#include "qemu/madvise.h"
#include "qemu/mprotect.h"
#include "qemu/hw-version.h"
#include "monitor/monitor.h"
static bool fips_enabled = false;
--
2.11.0
[PATCH v3 1/3] util/osdep: Avoid madvise proto on modern Solaris
On older Solaris releases (before Solaris 11), we didn't get a
prototype for madvise, and so util/osdep.c provides its own prototype.
Some time between the public Solaris 11.4 release and Solaris 11.4.42
CBE, we started getting an madvise prototype that looks like this:
extern int madvise(void *, size_t, int);
which conflicts with the prototype in util/osdeps.c. Instead of always
declaring this prototype, check if we're missing the madvise()
prototype, and only declare it ourselves if the prototype is missing.
Move the prototype to include/qemu/osdep.h, the normal place to handle
platform-specific header quirks.
The 'missing_madvise_proto' meson check contains an obviously wrong
prototype for madvise. So if that code compiles and links, we must be
missing the actual prototype for madvise.
Signed-off-by: Andrew Deason
---
Changes since v2:
- Rename new symbol to HAVE_MADVISE_WITHOUT_PROTOTYPE
- Move madvise prototype to include/qemu/osdep.h
- More comments in meson.build
Changes since v1:
- madvise prototype check changed to not be platforms-specific, and turned into
CONFIG_MADVISE_MISSING_PROTOTYPE.
include/qemu/osdep.h | 8
meson.build | 23 +--
util/osdep.c | 3 ---
3 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 322103aadb..f2274b24cb 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -393,20 +393,28 @@ void qemu_anon_ram_free(void *ptr, size_t size);
#if defined(__linux__) || defined(__FreeBSD__) || \
defined(__FreeBSD_kernel__) || defined(__DragonFly__)
#define HAVE_CHARDEV_PARPORT 1
#endif
#if defined(__HAIKU__)
#define SIGIO SIGPOLL
#endif
+#ifdef HAVE_MADVISE_WITHOUT_PROTOTYPE
+/*
+ * See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for discussion
+ * about Solaris missing the madvise() prototype.
+ */
+extern int madvise(char *, size_t, int);
+#endif
+
#if defined(CONFIG_LINUX)
#ifndef BUS_MCEERR_AR
#define BUS_MCEERR_AR 4
#endif
#ifndef BUS_MCEERR_AO
#define BUS_MCEERR_AO 5
#endif
#endif
#if defined(__linux__) && \
diff --git a/meson.build b/meson.build
index bae62efc9c..282e7c4650 100644
--- a/meson.build
+++ b/meson.build
@@ -1708,25 +1708,44 @@ config_host_data.set('CONFIG_EVENTFD', cc.links('''
int main(void) { return eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); }'''))
config_host_data.set('CONFIG_FDATASYNC', cc.links(gnu_source_prefix + '''
#include
int main(void) {
#if defined(_POSIX_SYNCHRONIZED_IO) && _POSIX_SYNCHRONIZED_IO > 0
return fdatasync(0);
#else
#error Not supported
#endif
}'''))
-config_host_data.set('CONFIG_MADVISE', cc.links(gnu_source_prefix + '''
+
+has_madvise = cc.links(gnu_source_prefix + '''
#include
#include
#include
- int main(void) { return madvise(NULL, 0, MADV_DONTNEED); }'''))
+ int main(void) { return madvise(NULL, 0, MADV_DONTNEED); }''')
+missing_madvise_proto = false
+if has_madvise
+ # Some platforms (illumos and Solaris before Solaris 11) provide madvise()
+ # but forget to prototype it. In this case, has_madvise will be true (the
+ # test program links despite a compile warning). To detect the
+ # missing-prototype case, we try again with a definitely-bogus prototype.
+ # This will only compile if the system headers don't provide the prototype;
+ # otherwise the conflicting prototypes will cause a compiler error.
+ missing_madvise_proto = cc.links(gnu_source_prefix + '''
+#include
+#include
+#include
+extern int madvise(int);
+int main(void) { return madvise(0); }''')
+endif
+config_host_data.set('CONFIG_MADVISE', has_madvise)
+config_host_data.set('HAVE_MADVISE_WITHOUT_PROTOTYPE', missing_madvise_proto)
+
config_host_data.set('CONFIG_MEMFD', cc.links(gnu_source_prefix + '''
#include
int main(void) { return memfd_create("foo", MFD_ALLOW_SEALING); }'''))
config_host_data.set('CONFIG_OPEN_BY_HANDLE', cc.links(gnu_source_prefix + '''
#include
#if !defined(AT_EMPTY_PATH)
# error missing definition
#else
int main(void) { struct file_handle fh; return open_by_handle_at(0, , 0);
}
#endif'''))
diff --git a/util/osdep.c b/util/osdep.c
index 7c4deda6fe..1825399bcf 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -21,23 +21,20 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
#include "qapi/error.h"
/* Needed early for CONFIG_BSD etc. */
#ifdef CONFIG_SOLARIS
#include
-/* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for
- discussion about Solaris header problems */
-extern int madvise(char *, size_t, int);
#endif
#include "qemu-common.h"
#include "qemu/cutils.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
#include "qemu/madvise.h"
#include "qemu/mprotect.h"
#include "qemu/hw-version.h"
#include "monitor/monitor.h"
--
2.11.0
[PATCH v3 0/3] Fixes for building on Solaris 11.4.42 CBE
With these minor fixes, I can build qemu on Solaris 11.4.42 CBE (Oracle's new rolling release thing), using '--disable-rdma --enable-modules --disable-dbus-display --target-list=x86_64-softmmu'. I'm just interested in the guest agent right now, so that's all I've tested (briefly), but the rest of the build wasn't hard to get working. With this, the guest agent runs fine using isa-serial. Changes since v2: - Rename new symbol to HAVE_MADVISE_WITHOUT_PROTOTYPE - Move madvise prototype to include/qemu/osdep.h - More comments in meson.build Changes since v1: - Change the CONFIG_MADVISE checks to not be platform-specific - Add the last commit removing util/osdep.c cruft Andrew Deason (3): util/osdep: Avoid madvise proto on modern Solaris hw/i386/acpi-build: Avoid 'sun' identifier util/osdep: Remove some early cruft hw/i386/acpi-build.c | 4 ++-- include/qemu/osdep.h | 8 meson.build | 23 +-- util/osdep.c | 10 -- 4 files changed, 31 insertions(+), 14 deletions(-) -- 2.11.0
Re: [PATCH v2 1/3] util/osdep: Avoid madvise proto on modern Solaris
On Tue, 15 Mar 2022 18:33:33 + Peter Maydell wrote: > Since this is a little bit tricky, I think a comment here will help > future readers: > > # Older Solaris/Illumos provide madvise() but forget to prototype it. I don't think it matters much, but just to mention, the prototype is in there, but it's deliberately hidden by some #ifdef logic for (older?) POSIX/XPG compliance or something. I sometimes try to phrase this in a way that reflects that, but it's hard so I probably won't care. > > +#ifdef HAVE_MADVISE_MISSING_PROTOTYPE > > /* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for > > discussion about Solaris header problems */ > > extern int madvise(char *, size_t, int); > > #endif > > As you note, this doesn't match the name we picked in meson.build. > I don't feel very strongly about the name (we certainly don't manage > consistency across the project about CONFIG_ vs HAVE_ !), but my suggestion > is HAVE_MADVISE_WITHOUT_PROTOTYPE. > > Can you put the prototype in include/qemu/osdep.h, please? > (Exact location not very important as long as it's inside > the extern-C block, but I suggest just under the bit where we > define SIGIO for __HAIKU__.) Okay, but this will cause callers that call madvise() directly to "work", even though they're not going through the qemu_madvise wrapper. There's one area in cross-platform code you noted before, in softmmu/physmem.c, and that does cause the same build error if the prototype is missing. (I'm going to add another commit to make that use the wrapper in the next patchset.) I assume that's not a concern unless I hear otherwise; just pointing it out. And all other comments will be addressed; thanks. -- Andrew Deason adea...@sinenomine.net
[PATCH v2 2/3] hw/i386/acpi-build: Avoid 'sun' identifier
On Solaris, 'sun' is #define'd to 1, which causes errors if a variable
is named 'sun'. Slightly change the name of the var for the Slot User
Number so we can build on Solaris.
Signed-off-by: Andrew Deason
---
hw/i386/acpi-build.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4ad4d7286c..dcf6ece3d0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -576,32 +576,32 @@ static void build_append_pci_bus_devices(Aml
*parent_scope, PCIBus *bus,
}
Aml *aml_pci_device_dsm(void)
{
Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
Aml *acpi_index = aml_local(0);
Aml *zero = aml_int(0);
Aml *bnum = aml_arg(4);
Aml *func = aml_arg(2);
Aml *rev = aml_arg(1);
-Aml *sun = aml_arg(5);
+Aml *sunum = aml_arg(5);
method = aml_method("PDSM", 6, AML_SERIALIZED);
/*
* PCI Firmware Specification 3.1
* 4.6. _DSM Definitions for PCI
*/
UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
ifctx = aml_if(aml_equal(aml_arg(0), UUID));
{
-aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
+aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sunum),
acpi_index));
ifctx1 = aml_if(aml_equal(func, zero));
{
uint8_t byte_list[1];
ifctx2 = aml_if(aml_equal(rev, aml_int(2)));
{
/*
* advertise function 7 if device has acpi-index
* acpi_index values:
*0: not present (default value)
--
2.11.0
Re: [PATCH v2 1/3] util/osdep: Avoid madvise proto on modern Solaris
On Mon, 14 Mar 2022 21:20:23 -0500 Andrew Deason wrote: > #ifdef CONFIG_SOLARIS > #include > +#endif > + > +#ifdef HAVE_MADVISE_MISSING_PROTOTYPE Oh goodness, that's not the right name is it. I'll wait a little bit to see if there are any other comments, but clearly that needs to be fixed. (I am testing that the meson checks behave as expected (by editing out the proto in mman.h), but I didn't run the whole build when doing that.) -- Andrew Deason adea...@sinenomine.net
[PATCH v2 3/3] util/osdep: Remove some early cruft
The include for statvfs.h has not been needed since all statvfs calls
were removed in commit 4a1418e07bdc ("Unbreak large mem support by
removing kqemu").
The comment mentioning CONFIG_BSD hasn't made sense since an include
for config-host.h was removed in commit aafd75841001 ("util: Clean up
includes").
Remove this cruft.
Signed-off-by: Andrew Deason
---
util/osdep.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/util/osdep.c b/util/osdep.c
index 560ce9111a..ed932a0c7c 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -17,26 +17,20 @@
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
#include "qapi/error.h"
-/* Needed early for CONFIG_BSD etc. */
-
-#ifdef CONFIG_SOLARIS
-#include
-#endif
-
#ifdef HAVE_MADVISE_MISSING_PROTOTYPE
/* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for
discussion about Solaris header problems */
extern int madvise(char *, size_t, int);
#endif
#include "qemu-common.h"
#include "qemu/cutils.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
--
2.11.0
[PATCH v2 1/3] util/osdep: Avoid madvise proto on modern Solaris
On older Solaris releases, we didn't get a protype for madvise, and so
util/osdep.c provides its own prototype. Some time between the public
Solaris 11.4 release and Solaris 11.4.42 CBE, we started getting an
madvise prototype that looks like this:
extern int madvise(void *, size_t, int);
which conflicts with the prototype in util/osdeps.c. Instead of always
declaring this prototype, check if we're missing the madvise()
prototype, and only declare it ourselves if the prototype is missing.
The 'missing_madvise_proto' meson check contains an obviously wrong
prototype for madvise. So if that code compiles and links, we must be
missing the actual prototype for madvise.
Signed-off-by: Andrew Deason
---
To be clear, I'm okay with removing the prototype workaround
unconditionally; I'm just not sure if there's enough consensus on doing
that.
The "missing prototype" check is based on getting a compiler error on a
conflicting prototype, since this just seems more precise and certain
than getting an error from a missing prototype (needs
-Werror=missing-prototypes or -Werror). But I can do it the other way
around if needed.
Changes since v1:
- madvise prototype check changed to not be platforms-specific, and turned into
CONFIG_MADVISE_MISSING_PROTOTYPE.
meson.build | 17 +++--
util/osdep.c | 3 +++
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index 2d6601467f..ff5fce693e 100644
--- a/meson.build
+++ b/meson.build
@@ -1705,25 +1705,38 @@ config_host_data.set('CONFIG_EVENTFD', cc.links('''
int main(void) { return eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); }'''))
config_host_data.set('CONFIG_FDATASYNC', cc.links(gnu_source_prefix + '''
#include
int main(void) {
#if defined(_POSIX_SYNCHRONIZED_IO) && _POSIX_SYNCHRONIZED_IO > 0
return fdatasync(0);
#else
#error Not supported
#endif
}'''))
-config_host_data.set('CONFIG_MADVISE', cc.links(gnu_source_prefix + '''
+
+has_madvise = cc.links(gnu_source_prefix + '''
#include
#include
#include
- int main(void) { return madvise(NULL, 0, MADV_DONTNEED); }'''))
+ int main(void) { return madvise(NULL, 0, MADV_DONTNEED); }''')
+missing_madvise_proto = false
+if has_madvise
+ missing_madvise_proto = cc.links(gnu_source_prefix + '''
+#include
+#include
+#include
+extern int madvise(int);
+int main(void) { return madvise(0); }''')
+endif
+config_host_data.set('CONFIG_MADVISE', has_madvise)
+config_host_data.set('CONFIG_MADVISE_MISSING_PROTOTYPE', missing_madvise_proto)
+
config_host_data.set('CONFIG_MEMFD', cc.links(gnu_source_prefix + '''
#include
int main(void) { return memfd_create("foo", MFD_ALLOW_SEALING); }'''))
config_host_data.set('CONFIG_OPEN_BY_HANDLE', cc.links(gnu_source_prefix + '''
#include
#if !defined(AT_EMPTY_PATH)
# error missing definition
#else
int main(void) { struct file_handle fh; return open_by_handle_at(0, , 0);
}
#endif'''))
diff --git a/util/osdep.c b/util/osdep.c
index 7c4deda6fe..560ce9111a 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -21,20 +21,23 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
#include "qapi/error.h"
/* Needed early for CONFIG_BSD etc. */
#ifdef CONFIG_SOLARIS
#include
+#endif
+
+#ifdef HAVE_MADVISE_MISSING_PROTOTYPE
/* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for
discussion about Solaris header problems */
extern int madvise(char *, size_t, int);
#endif
#include "qemu-common.h"
#include "qemu/cutils.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
#include "qemu/madvise.h"
--
2.11.0
[PATCH v2 0/3] Fixes for building on Solaris 11.4.42 CBE
With these minor fixes, I can build qemu on Solaris 11.4.42 CBE (Oracle's new rolling release thing), using '--disable-rdma --enable-modules --disable-dbus-display --target-list=x86_64-softmmu'. I'm just interested in the guest agent right now, so that's all I've tested (briefly), but the rest of the build wasn't hard to get working. With this, the guest agent runs fine using isa-serial. Changes since v1: - Change the CONFIG_MADVISE checks to not be platform-specific - Add the last commit removing util/osdep.c cruft Andrew Deason (3): util/osdep: Avoid madvise proto on modern Solaris hw/i386/acpi-build: Avoid 'sun' identifier util/osdep: Remove some early cruft hw/i386/acpi-build.c | 4 ++-- meson.build | 17 +++-- util/osdep.c | 5 + 3 files changed, 18 insertions(+), 8 deletions(-) -- 2.11.0
Re: [PATCH 1/2] util/osdep: Avoid madvise proto on modern Solaris
On Mon, 14 Mar 2022 19:01:06 + Daniel P. Berrangé wrote: > We have a general purpose platform support policy > > https://www.qemu.org/docs/master/about/build-platforms.html > > where the common rule ends up being "the current major release, > and the previous major release for 2 years overlap". > > The question is what counts as a major release from a Solaris > POV ? In terms of long life distros, our policy gives about > 4-5 years of supportable life in the best case. I wouldn't > want to go beyond that ballpark for Solaris. Can we come up > with an interpration of our policy to map to Solaris that > doesn't tie our hands for longer than 4-5 years worst case. FWIW, some relevant Solaris version numbers, as I understand it: 11.4.42 CBE: public release March 2022. (Non-production use only, rolling release schedule.) 11.4: public release August 2018. 11.3: public release October 2015. Going by the minor version number (11.3 vs 11.4) sounds similar to Linux distros; they've come out every few years in the past. But I don't know how this is going to look with the CBE stuff in the future, or if anyone knows (it's very new). > IOW, we certainly do NOT need to support arbitrarily old > Solaris. If madvise has done what we need for 4-5 years, > then we can likely not need to test for it, and just assume > its existance. This just requires someone to specify how > we interpret our build platform policy to exclude older > Solaris. Specifically for the madvise() prototype workaround, I looked a little bit into what version changes actually matter. I think all of Solaris 11 is probably okay without the workaround (I can check back to Solaris 11.1, but just by looking at the mman.h header, not actually testing a build), because we specify -D__EXTENSIONS__. Illumos and Solaris 10 look like they would need the workaround. So practically speaking for this patchset, it seems more like a question of Illumos support. -- Andrew Deason adea...@sinenomine.net
Re: [PATCH 1/2] util/osdep: Avoid madvise proto on modern Solaris
On Mon, 14 Mar 2022 16:36:00 + Peter Maydell wrote: > On Mon, 14 Mar 2022 at 16:12, Andrew Deason wrote: > > #ifdef CONFIG_SOLARIS > > #include > > +#ifndef HAVE_MADVISE_PROTO > > /* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for > > discussion about Solaris header problems */ > > extern int madvise(char *, size_t, int); > > #endif > > +#endif > > Rather than keeping this inside a CONFIG_SOLARIS and only doing > the meson.build test if targetos == sunos, I would prefer it if we > unconditionally determined two things in meson.build: > (1) do we have madvise in the usual way? (this is what we would > want CONFIG_MADVISE to be, and might even be what it actually is) > (2) do we have madvise but only if we provide a prototype for it > ourselves? (maybe CONFIG_MADVISE_NO_PROTO) CONFIG_MADVISE is set if we can cc.links() something that calls madvise(). If we're missing the prototype, that will fail with -Werror, but I expect succeeds otherwise. If cc.links() just uses the cflags for the build, then it seems like it might succeed or fail depending on --enable-werror. I see some other tests give -Werror as an explicit extra argument (HAVE_BROKEN_SIZE_MAX, and something for fuzzing); should this be doing the same to make sure it fails for a missing prototype? Also just to mention, if we don't care about older Solaris, the prototype can just be unconditionally removed. It's pretty annoying to even try to build qemu from git on Solaris 11.4 and earlier, because many of the build requirements need to be installed/compiled manually (notably python 3.6+, but iirc also ninja, meson, etc). So I haven't really tried; there may be many other build issues there. > Side note: do you know why CONFIG_SOLARIS includes sys/statvfs.h ? > Is that unrelated to madvise() ? I think so, it was added in commit 605686cd7ac, which predates madvise() in that file. It does look like it could be removed from a quick glance. Would you want me to add a commit to remove it? (Assuming it still compiles okay.) Or just do that in the same commit as changing the madvise prototype logic? -- Andrew Deason adea...@sinenomine.net
[PATCH 0/2] Fixes for building on Solaris 11.4.42 CBE
With these minor fixes, I can build qemu on Solaris 11.4.42 CBE (Oracle's new rolling release thing), using '--disable-rdma --enable-modules --disable-dbus-display --target-list=x86_64-softmmu'. I'm just interested in the guest agent right now, so that's all I've tested (briefly), but the rest of the build wasn't hard to get working. With this, the guest agent runs fine using isa-serial. Andrew Deason (2): util/osdep: Avoid madvise proto on modern Solaris hw/i386/acpi-build: Avoid 'sun' identifier hw/i386/acpi-build.c | 4 ++-- meson.build | 3 +++ util/osdep.c | 2 ++ 3 files changed, 7 insertions(+), 2 deletions(-) -- 2.11.0
[PATCH 1/2] util/osdep: Avoid madvise proto on modern Solaris
On older Solaris releases, we didn't get a protype for madvise, and so
util/osdep.c provides its own prototype. Some time between the public
Solaris 11.4 release and Solaris 11.4.42 CBE, we started getting an
madvise prototype that looks like this:
extern int madvise(void *, size_t, int);
Which conflicts with the prototype in util/osdeps.c. Instead of always
declaring this prototype, check if madvise() is already declared, so
we don't need to declare it ourselves.
Signed-off-by: Andrew Deason
---
I'm not sure if a test is needed for this at all; that is, how much qemu
cares about earlier Solaris. The madvise prototype exists earlier in
Solaris 11 (I'm not sure when it started appearing usefully), but in
11.4 and earlier it was compatible with the char* prototype.
meson.build | 3 +++
util/osdep.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/meson.build b/meson.build
index 2d6601467f..c29d49c7a1 100644
--- a/meson.build
+++ b/meson.build
@@ -322,20 +322,23 @@ if targetos == 'windows'
include_directories:
include_directories('.'))
host_dsosuf = '.dll'
elif targetos == 'darwin'
coref = dependency('appleframeworks', modules: 'CoreFoundation')
iokit = dependency('appleframeworks', modules: 'IOKit', required: false)
host_dsosuf = '.dylib'
elif targetos == 'sunos'
socket = [cc.find_library('socket'),
cc.find_library('nsl'),
cc.find_library('resolv')]
+ config_host_data.set('HAVE_MADVISE_PROTO',
+ cc.has_function('madvise',
+ prefix: '#include '))
elif targetos == 'haiku'
socket = [cc.find_library('posix_error_mapper'),
cc.find_library('network'),
cc.find_library('bsd')]
elif targetos == 'openbsd'
if get_option('tcg').allowed() and target_dirs.length() > 0
# Disable OpenBSD W^X if available
emulator_link_args = cc.get_supported_link_arguments('-Wl,-z,wxneeded')
endif
endif
diff --git a/util/osdep.c b/util/osdep.c
index 7c4deda6fe..c99083372b 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -21,24 +21,26 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
#include "qapi/error.h"
/* Needed early for CONFIG_BSD etc. */
#ifdef CONFIG_SOLARIS
#include
+#ifndef HAVE_MADVISE_PROTO
/* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for
discussion about Solaris header problems */
extern int madvise(char *, size_t, int);
#endif
+#endif
#include "qemu-common.h"
#include "qemu/cutils.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
#include "qemu/madvise.h"
#include "qemu/mprotect.h"
#include "qemu/hw-version.h"
#include "monitor/monitor.h"
--
2.11.0
[PATCH 2/2] hw/i386/acpi-build: Avoid 'sun' identifier
On Solaris, 'sun' is #define'd to 1, which causes errors if a variable
is named 'sun'. Slightly change the name of the var for the Slot User
Number so we can build on Solaris.
Signed-off-by: Andrew Deason
---
hw/i386/acpi-build.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4ad4d7286c..dcf6ece3d0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -576,32 +576,32 @@ static void build_append_pci_bus_devices(Aml
*parent_scope, PCIBus *bus,
}
Aml *aml_pci_device_dsm(void)
{
Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
Aml *acpi_index = aml_local(0);
Aml *zero = aml_int(0);
Aml *bnum = aml_arg(4);
Aml *func = aml_arg(2);
Aml *rev = aml_arg(1);
-Aml *sun = aml_arg(5);
+Aml *sunum = aml_arg(5);
method = aml_method("PDSM", 6, AML_SERIALIZED);
/*
* PCI Firmware Specification 3.1
* 4.6. _DSM Definitions for PCI
*/
UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
ifctx = aml_if(aml_equal(aml_arg(0), UUID));
{
-aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
+aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sunum),
acpi_index));
ifctx1 = aml_if(aml_equal(func, zero));
{
uint8_t byte_list[1];
ifctx2 = aml_if(aml_equal(rev, aml_int(2)));
{
/*
* advertise function 7 if device has acpi-index
* acpi_index values:
*0: not present (default value)
--
2.11.0
