Move the declaration of the struct into 'qemu_monitor_priv.h' as other code has no business in peeking into the monitor messages. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 14 -------------- src/qemu/qemu_monitor_json.c | 3 +++ src/qemu/qemu_monitor_priv.h | 16 ++++++++++++++++ tests/qemucapsprobemock.c | 3 +++ 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 12005ac624..1d21183d82 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -34,21 +34,7 @@ #include "virenum.h" typedef struct _qemuMonitor qemuMonitor; - typedef struct _qemuMonitorMessage qemuMonitorMessage; -struct _qemuMonitorMessage { - int txFD; - - const char *txBuffer; - int txOffset; - int txLength; - - /* Used by the JSON monitor to hold reply / error */ - void *rxObject; - - /* True if rxObject is ready, or a fatal error occurred on the monitor channel */ - bool finished; -}; typedef enum { QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE = 0, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5bc1b2342c..1de932f638 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -44,6 +44,9 @@ # include "libvirt_qemu_probes.h" #endif +#define LIBVIRT_QEMU_MONITOR_PRIV_H_ALLOW +#include "qemu_monitor_priv.h" + #define VIR_FROM_THIS VIR_FROM_QEMU VIR_LOG_INIT("qemu.qemu_monitor_json"); diff --git a/src/qemu/qemu_monitor_priv.h b/src/qemu/qemu_monitor_priv.h index 31bb3526b9..6115f830de 100644 --- a/src/qemu/qemu_monitor_priv.h +++ b/src/qemu/qemu_monitor_priv.h @@ -24,5 +24,21 @@ #include "qemu_monitor.h" + +struct _qemuMonitorMessage { + int txFD; + + const char *txBuffer; + int txOffset; + int txLength; + + /* Used by the JSON monitor to hold reply / error */ + void *rxObject; + + /* True if rxObject is ready, or a fatal error occurred on the monitor channel */ + bool finished; +}; + + void qemuMonitorResetCommandID(qemuMonitor *mon); diff --git a/tests/qemucapsprobemock.c b/tests/qemucapsprobemock.c index 915036d178..2717ed5d84 100644 --- a/tests/qemucapsprobemock.c +++ b/tests/qemucapsprobemock.c @@ -25,6 +25,9 @@ #include "qemu/qemu_monitor.h" #include "qemu/qemu_monitor_json.h" +#define LIBVIRT_QEMU_MONITOR_PRIV_H_ALLOW +#include "qemu/qemu_monitor_priv.h" + #define REAL_SYM(realFunc) \ do { \ if (!realFunc && !(realFunc = dlsym(RTLD_NEXT, __FUNCTION__))) { \ -- 2.34.1

In order to mock the SCM_RIGHTS sendmsg to simulate sending filedescriptors to fake qemu in tests we need access to some fields of 'struct _qemuMonitor'. Move it's declaration to the private header file. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 50 --------------------------------- src/qemu/qemu_monitor_priv.h | 54 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 3de04eb1cc..327d6d978a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -65,56 +65,6 @@ VIR_LOG_INIT("qemu.qemu_monitor"); */ #define QEMU_MONITOR_MAX_RESPONSE (10 * 1024 * 1024) -struct _qemuMonitor { - virObjectLockable parent; - - virCond notify; - - int fd; - - GMainContext *context; - GSocket *socket; - GSource *watch; - - virDomainObj *vm; - char *domainName; - - qemuMonitorCallbacks *cb; - void *callbackOpaque; - - /* If there's a command being processed this will be - * non-NULL */ - qemuMonitorMessage *msg; - - /* Buffer incoming data ready for Text/QMP monitor - * code to process & find message boundaries */ - size_t bufferOffset; - size_t bufferLength; - char *buffer; - - /* If anything went wrong, this will be fed back - * the next monitor msg */ - virError lastError; - - /* Set to true when EOF is detected on the monitor */ - bool goteof; - - int nextSerial; - - bool waitGreeting; - - /* If found, path to the virtio memballoon driver */ - char *balloonpath; - bool ballooninit; - - /* Log file context of the qemu process to dig for usable info */ - qemuMonitorReportDomainLogError logFunc; - void *logOpaque; - virFreeCallback logDestroy; - - /* true if qemu no longer wants 'props' sub-object of object-add */ - bool objectAddNoWrap; -}; /** * QEMU_CHECK_MONITOR_FULL: diff --git a/src/qemu/qemu_monitor_priv.h b/src/qemu/qemu_monitor_priv.h index 6115f830de..606aa79fbd 100644 --- a/src/qemu/qemu_monitor_priv.h +++ b/src/qemu/qemu_monitor_priv.h @@ -24,6 +24,8 @@ #include "qemu_monitor.h" +#include <gio/gio.h> + struct _qemuMonitorMessage { int txFD; @@ -40,5 +42,57 @@ struct _qemuMonitorMessage { }; +struct _qemuMonitor { + virObjectLockable parent; + + virCond notify; + + int fd; + + GMainContext *context; + GSocket *socket; + GSource *watch; + + virDomainObj *vm; + char *domainName; + + qemuMonitorCallbacks *cb; + void *callbackOpaque; + + /* If there's a command being processed this will be + * non-NULL */ + qemuMonitorMessage *msg; + + /* Buffer incoming data ready for Text/QMP monitor + * code to process & find message boundaries */ + size_t bufferOffset; + size_t bufferLength; + char *buffer; + + /* If anything went wrong, this will be fed back + * the next monitor msg */ + virError lastError; + + /* Set to true when EOF is detected on the monitor */ + bool goteof; + + int nextSerial; + + bool waitGreeting; + + /* If found, path to the virtio memballoon driver */ + char *balloonpath; + bool ballooninit; + + /* Log file context of the qemu process to dig for usable info */ + qemuMonitorReportDomainLogError logFunc; + void *logOpaque; + virFreeCallback logDestroy; + + /* true if qemu no longer wants 'props' sub-object of object-add */ + bool objectAddNoWrap; +}; + + void qemuMonitorResetCommandID(qemuMonitor *mon); -- 2.34.1

Adding an exception for the whole file usually defeats the purpose of a syntax check and is also likely to get forgotten once the file is removed. In case of the suggestion of using 'safewrite' instead of write even the comment for safewrite states that the function needs to be used only in certain cases. Remove the blanket exceptions for files and use an exclude string instead. The only instance where we keep the full file exception is for src/libvirt-stream.c as there are multiple uses in example code in comments where I couldn't find a nicer targetted wapproach. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- build-aux/syntax-check.mk | 6 ++---- src/locking/lock_daemon.c | 4 ++-- src/logging/log_daemon.c | 4 ++-- src/lxc/lxc_controller.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/remote/remote_ssh_helper.c | 2 +- src/rpc/virnetsocket.c | 4 ++-- src/util/vircommand.c | 4 ++-- src/util/virfdstream.c | 2 +- src/util/virfile.c | 10 +++++----- tests/shunloadtest.c | 2 +- tests/vircgroupmock.c | 2 +- tests/virnettlssessiontest.c | 2 +- tools/virsh-console.c | 2 +- 14 files changed, 23 insertions(+), 25 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index b96d126bdc..9407581d0e 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -116,6 +116,7 @@ VC_LIST_ALWAYS_EXCLUDE_REGEX = \ # the safewrite wrapper. sc_avoid_write: @prohibit='\<write *\(' \ + exclude='sc_avoid_write' \ in_vc_files='\.c$$' \ halt='consider using safewrite instead of write' \ $(_sc_search_regexp) @@ -1569,10 +1570,7 @@ sc_prohibit_enum_impl_with_vir_prefix_in_virsh: # List all syntax-check exemptions: exclude_file_name_regexp--sc_avoid_strcase = ^tools/vsh\.h$$ -_src1=libvirt-stream|qemu/qemu_monitor|util/vir(command|file|fdstream)|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon|logging/log_daemon|remote/remote_ssh_helper -_test1=shunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupmock|commandhelper -exclude_file_name_regexp--sc_avoid_write = \ - ^(src/($(_src1))|tools/virsh-console|tests/($(_test1)))\.c$$ +exclude_file_name_regexp--sc_avoid_write = ^src/libvirt-stream\.c$$ exclude_file_name_regexp--sc_bindtextdomain = .* diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index b44649bfbe..455bb15d1d 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1104,7 +1104,7 @@ int main(int argc, char **argv) { */ if (statuswrite != -1) { char status = 0; - while (write(statuswrite, &status, 1) == -1 && + while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */ errno == EINTR) ; VIR_FORCE_CLOSE(statuswrite); @@ -1133,7 +1133,7 @@ int main(int argc, char **argv) { if (ret != 0) { /* Tell parent of daemon what failed */ char status = ret; - while (write(statuswrite, &status, 1) == -1 && + while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */ errno == EINTR) ; } diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 245df9dbbd..4c6118a980 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -910,7 +910,7 @@ int main(int argc, char **argv) { */ if (statuswrite != -1) { char status = 0; - while (write(statuswrite, &status, 1) == -1 && + while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */ errno == EINTR) ; VIR_FORCE_CLOSE(statuswrite); @@ -939,7 +939,7 @@ int main(int argc, char **argv) { if (ret != 0) { /* Tell parent of daemon what failed */ char status = ret; - while (write(statuswrite, &status, 1) == -1 && + while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */ errno == EINTR) ; } diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 7e798f7b3f..677fa5a4fb 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1222,7 +1222,7 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaqu } rewrite: - done = write(fd, buf, *len); + done = write(fd, buf, *len); /* sc_avoid_write */ if (done == -1 && errno == EINTR) goto rewrite; if (done == -1 && errno != EAGAIN) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 327d6d978a..f514998ba5 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -344,7 +344,7 @@ qemuMonitorIOWrite(qemuMonitor *mon) buf = mon->msg->txBuffer + mon->msg->txOffset; len = mon->msg->txLength - mon->msg->txOffset; if (mon->msg->txFD == -1) - done = write(mon->fd, buf, len); + done = write(mon->fd, buf, len); /* sc_avoid_write */ else done = qemuMonitorIOWriteWithFD(mon, buf, len, mon->msg->txFD); diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c index 6403fe1761..b4735027be 100644 --- a/src/remote/remote_ssh_helper.c +++ b/src/remote/remote_ssh_helper.c @@ -255,7 +255,7 @@ virRemoteSSHHelperEventOnStdout(int watch G_GNUC_UNUSED, if (events & VIR_EVENT_HANDLE_WRITABLE && proxy->sockToTerminal.offset) { ssize_t done; - done = write(fd, + done = write(fd, /* sc_avoid_write */ proxy->sockToTerminal.data, proxy->sockToTerminal.offset); if (done < 0) { diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 51cab4f80c..1af2778b97 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1623,7 +1623,7 @@ static ssize_t virNetSocketTLSSessionWrite(const char *buf, void *opaque) { virNetSocket *sock = opaque; - return write(sock->fd, buf, len); + return write(sock->fd, buf, len); /* sc_avoid_write */ } @@ -1818,7 +1818,7 @@ static ssize_t virNetSocketWriteWire(virNetSocket *sock, const char *buf, size_t VIR_NET_TLS_HANDSHAKE_COMPLETE) { ret = virNetTLSSessionWrite(sock->tlsSession, buf, len); } else { - ret = write(sock->fd, buf, len); + ret = write(sock->fd, buf, len); /* sc_avoid_write */ } if (ret < 0) { diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 3b8c1c65c1..41cf552d7b 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1794,7 +1794,7 @@ virCommandSendBuffersHandlePoll(virCommand *cmd, if (i == virCommandGetNumSendBuffers(cmd)) return 0; - done = write(fds->fd, + done = write(fds->fd, /* sc_avoid_write */ cmd->sendBuffers[i].buffer + cmd->sendBuffers[i].offset, cmd->sendBuffers[i].buflen - cmd->sendBuffers[i].offset); if (done < 0) { @@ -2306,7 +2306,7 @@ virCommandProcessIO(virCommand *cmd) fds[i].fd == cmd->inpipe) { int done; - done = write(cmd->inpipe, cmd->inbuf + inoff, + done = write(cmd->inpipe, cmd->inbuf + inoff, /* sc_avoid_write */ inlen - inoff); if (done < 0) { if (errno == EPIPE) { diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c index e2c6461ded..b596659702 100644 --- a/src/util/virfdstream.c +++ b/src/util/virfdstream.c @@ -850,7 +850,7 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes) ret = nbytes; } else { retry: - ret = write(fdst->fd, bytes, nbytes); + ret = write(fdst->fd, bytes, nbytes); /* sc_avoid_write */ if (ret < 0) { VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR if (errno == EAGAIN || errno == EWOULDBLOCK) { diff --git a/src/util/virfile.c b/src/util/virfile.c index 5d6f14ba7e..a04f888e06 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1112,17 +1112,17 @@ saferead(int fd, void *buf, size_t count) return nread; } -/* Like write(), but restarts after EINTR. Doesn't play - * nicely with nonblocking FD and EAGAIN, in which case - * you want to use bare write(). Or even use virSocket() - * if the FD is related to a socket rather than a plain +/* Like write(), but restarts after EINTR. Encouraged by sc_avoid_write. + * Doesn't play nicely with nonblocking FD and EAGAIN, in which case + * you want to use bare write() and mark it's use with sc_avoid_write. + * Or even use virSocket() if the FD is related to a socket rather than a plain * file or pipe. */ ssize_t safewrite(int fd, const void *buf, size_t count) { size_t nwritten = 0; while (count > 0) { - ssize_t r = write(fd, buf, count); + ssize_t r = write(fd, buf, count); /* sc_avoid_write */ if (r < 0 && errno == EINTR) continue; diff --git a/tests/shunloadtest.c b/tests/shunloadtest.c index d4ab3cd5ac..724532a3ea 100644 --- a/tests/shunloadtest.c +++ b/tests/shunloadtest.c @@ -81,7 +81,7 @@ static void *threadMain(void *arg) static void sigHandler(int sig) { - ignore_value(write(STDERR_FILENO, "FAIL\n", 5)); + ignore_value(write(STDERR_FILENO, "FAIL\n", 5)); /* sc_avoid_write */ signal(sig, SIG_DFL); raise(sig); } diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index a67c0d95d7..cebf1912f7 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -83,7 +83,7 @@ static int make_file(const char *path, if ((fd = real_open(filepath, O_CREAT|O_WRONLY, 0600)) < 0) goto cleanup; - if (write(fd, value, strlen(value)) != strlen(value)) + if (write(fd, value, strlen(value)) != strlen(value)) /* sc_avoid_write */ goto cleanup; ret = 0; diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index b9249cca56..9e0d6a46e6 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -54,7 +54,7 @@ static ssize_t testWrite(const char *buf, size_t len, void *opaque) { int *fd = opaque; - return write(*fd, buf, len); + return write(*fd, buf, len); /* sc_avoid_write */ } static ssize_t testRead(char *buf, size_t len, void *opaque) diff --git a/tools/virsh-console.c b/tools/virsh-console.c index 67ee608706..b8780c714d 100644 --- a/tools/virsh-console.c +++ b/tools/virsh-console.c @@ -316,7 +316,7 @@ virConsoleEventOnStdout(int watch G_GNUC_UNUSED, con->streamToTerminal.offset) { ssize_t done; size_t avail; - done = write(fd, + done = write(fd, /* sc_avoid_write */ con->streamToTerminal.data, con->streamToTerminal.offset); if (done < 0) { -- 2.34.1

All modern QEMU versions use FD passing for listening unix sockets so the test should reflect this. This will later help when removing the legacy code paths when we drop support for old QEMUs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuhotplugtest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index ce215da099..eb7f5ed961 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -95,6 +95,7 @@ qemuHotplugCreateObjects(virDomainXMLOption *xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SCSI_BLOCK); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_USB_KBD); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_NETDEV_VHOST_VDPA); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; @@ -764,6 +765,7 @@ mymain(void) "object-del", QMP_OK); DO_TEST_ATTACH("base-live", "qemu-agent", false, true, + "getfd", QMP_OK, "chardev-add", QMP_OK, "device_add", QMP_OK); DO_TEST_DETACH("base-live", "qemu-agent-detach", false, false, @@ -856,6 +858,7 @@ mymain(void) "device_del", QMP_DEVICE_DELETED("ua-UserWatchdog") QMP_OK); DO_TEST_ATTACH("base-live", "guestfwd", false, true, + "getfd", QMP_OK, "chardev-add", QMP_OK, "netdev_add", QMP_OK); DO_TEST_DETACH("base-live", "guestfwd", false, false, -- 2.34.1