| Message ID | 1620390320-301716-7-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Live Update | expand |
On 5/7/21 7:25 AM, Steve Sistare wrote: > Add a qemu_exec_requested() hook that causes the main loop to exit and > re-exec qemu using the same initial arguments. If /usr/bin/qemu-exec > exists, exec that instead. This is an optional site-specific trampoline > that may alter the environment before exec'ing the qemu binary. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > --- > +static void qemu_exec(void) > +{ > + const char *helper = "/usr/bin/qemu-exec"; > + const char *bin = !access(helper, X_OK) ? helper : argv_main[0]; Reads awkwardly; I would have used '...= access(helper, X_OK) == 0 ?...' > + > + execvp(bin, argv_main); > + error_report("execvp failed, errno %d.", errno); error_report should not be used with a trailing dot. Also, %d for errno is awkward, better is: error_report("execvp failed: %s", strerror(errno)); > + exit(1); We aren't consistent about use of EXIT_FAILED.
On Fri, May 07, 2021 at 05:25:04AM -0700, Steve Sistare wrote: > @@ -660,6 +673,16 @@ void qemu_system_debug_request(void) > qemu_notify_event(); > } > > +static void qemu_exec(void) > +{ > + const char *helper = "/usr/bin/qemu-exec"; The network up script is get_relocated_path(CONFIG_SYSCONFDIR "/qemu-ifup"). For consistency maybe this should use the same path rather than /usr/bin/.
On 5/7/2021 10:31 AM, Eric Blake wrote: > On 5/7/21 7:25 AM, Steve Sistare wrote: >> Add a qemu_exec_requested() hook that causes the main loop to exit and >> re-exec qemu using the same initial arguments. If /usr/bin/qemu-exec >> exists, exec that instead. This is an optional site-specific trampoline >> that may alter the environment before exec'ing the qemu binary. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> --- > >> +static void qemu_exec(void) >> +{ >> + const char *helper = "/usr/bin/qemu-exec"; >> + const char *bin = !access(helper, X_OK) ? helper : argv_main[0]; > > Reads awkwardly; I would have used '...= access(helper, X_OK) == 0 ?...' Will fix. >> + >> + execvp(bin, argv_main); >> + error_report("execvp failed, errno %d.", errno); > > error_report should not be used with a trailing dot. Will fix. I was not sure because I see examples both ways, though no dot prevails. Perhaps it should be added to the style guide and checkpatch. > Also, %d for errno is awkward, better is: > > error_report("execvp failed: %s", strerror(errno)); I shy away from strerror because it is not thread safe, but I see qemu uses it extensively. Will fix. > >> + exit(1); > > We aren't consistent about use of EXIT_FAILED. OK, I will use EXIT_FAILURE. Thanks for reviewing. - Steve
On 5/12/2021 12:27 PM, Stefan Hajnoczi wrote: > On Fri, May 07, 2021 at 05:25:04AM -0700, Steve Sistare wrote: >> @@ -660,6 +673,16 @@ void qemu_system_debug_request(void) >> qemu_notify_event(); >> } >> >> +static void qemu_exec(void) >> +{ >> + const char *helper = "/usr/bin/qemu-exec"; > > The network up script is get_relocated_path(CONFIG_SYSCONFDIR > "/qemu-ifup"). For consistency maybe this should use the same path > rather than /usr/bin/. CONFIG_QEMU_HELPERDIR=/usr/libexec looks like a good choice. And maybe rename qemu-exec to qemu-exec-helper, analogous to qemu-bridge-helper. - Steve
On Thu, May 13, 2021 at 04:19:22PM -0400, Steven Sistare wrote: > On 5/7/2021 10:31 AM, Eric Blake wrote: > > On 5/7/21 7:25 AM, Steve Sistare wrote: > >> Add a qemu_exec_requested() hook that causes the main loop to exit and > >> re-exec qemu using the same initial arguments. If /usr/bin/qemu-exec > >> exists, exec that instead. This is an optional site-specific trampoline > >> that may alter the environment before exec'ing the qemu binary. > >> > >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > >> --- > > > >> +static void qemu_exec(void) > >> +{ > >> + const char *helper = "/usr/bin/qemu-exec"; > >> + const char *bin = !access(helper, X_OK) ? helper : argv_main[0]; > > > > Reads awkwardly; I would have used '...= access(helper, X_OK) == 0 ?...' > > Will fix. > > >> + > >> + execvp(bin, argv_main); > >> + error_report("execvp failed, errno %d.", errno); > > > > error_report should not be used with a trailing dot. > > Will fix. I was not sure because I see examples both ways, though no dot prevails. > Perhaps it should be added to the style guide and checkpatch. > > > Also, %d for errno is awkward, better is: > > > > error_report("execvp failed: %s", strerror(errno)); > > I shy away from strerror because it is not thread safe, but I see qemu uses it > extensively. Will fix. GLib provides 'g_strerror' which is threadsafe, but without the horrible API of strerror_r. It works by caching the errno strings in a static table on demand. We don't use it much in QEMU, but I think we ought to. Regards, Daniel
diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h index a535691..50c84af 100644 --- a/include/sysemu/runstate.h +++ b/include/sysemu/runstate.h @@ -56,6 +56,7 @@ void qemu_system_wakeup_enable(WakeupReason reason, bool enabled); void qemu_register_wakeup_notifier(Notifier *notifier); void qemu_register_wakeup_support(void); void qemu_system_shutdown_request(ShutdownCause reason); +void qemu_system_exec_request(void); void qemu_system_powerdown_request(void); void qemu_register_powerdown_notifier(Notifier *notifier); void qemu_register_shutdown_notifier(Notifier *notifier); diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 8fae667..f56058e 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -9,6 +9,7 @@ /* vl.c */ extern int only_migratable; +extern char **argv_main; extern const char *qemu_name; extern QemuUUID qemu_uuid; extern bool qemu_uuid_set; diff --git a/softmmu/globals.c b/softmmu/globals.c index 7d0fc81..2bb630d 100644 --- a/softmmu/globals.c +++ b/softmmu/globals.c @@ -60,6 +60,7 @@ bool boot_strict; uint8_t *boot_splash_filedata; int only_migratable; /* turn it off unless user states otherwise */ int icount_align_option; +char **argv_main; /* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the * little-endian "wire format" described in the SMBIOS 2.6 specification. diff --git a/softmmu/runstate.c b/softmmu/runstate.c index ce8977c..bea7513 100644 --- a/softmmu/runstate.c +++ b/softmmu/runstate.c @@ -338,6 +338,7 @@ static ShutdownCause reset_requested; static ShutdownCause shutdown_requested; static int shutdown_signal; static pid_t shutdown_pid; +static int exec_requested; static int powerdown_requested; static int debug_requested; static int suspend_requested; @@ -367,6 +368,11 @@ static int qemu_shutdown_requested(void) return qatomic_xchg(&shutdown_requested, SHUTDOWN_CAUSE_NONE); } +static int qemu_exec_requested(void) +{ + return qatomic_xchg(&exec_requested, 0); +} + static void qemu_kill_report(void) { if (!qtest_driver() && shutdown_signal) { @@ -625,6 +631,13 @@ void qemu_system_shutdown_request(ShutdownCause reason) qemu_notify_event(); } +void qemu_system_exec_request(void) +{ + shutdown_requested = 1; + exec_requested = 1; + qemu_notify_event(); +} + static void qemu_system_powerdown(void) { qapi_event_send_powerdown(); @@ -660,6 +673,16 @@ void qemu_system_debug_request(void) qemu_notify_event(); } +static void qemu_exec(void) +{ + const char *helper = "/usr/bin/qemu-exec"; + const char *bin = !access(helper, X_OK) ? helper : argv_main[0]; + + execvp(bin, argv_main); + error_report("execvp failed, errno %d.", errno); + exit(1); +} + static bool main_loop_should_exit(void) { RunState r; @@ -673,6 +696,11 @@ static bool main_loop_should_exit(void) } request = qemu_shutdown_requested(); if (request) { + + if (qemu_exec_requested()) { + qemu_exec(); + /* not reached */ + } qemu_kill_report(); qemu_system_shutdown(request); if (shutdown_action == SHUTDOWN_ACTION_PAUSE) { diff --git a/softmmu/vl.c b/softmmu/vl.c index aadb526..04ab752 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2662,6 +2662,7 @@ void qemu_init(int argc, char **argv, char **envp) error_init(argv[0]); qemu_init_exec_dir(argv[0]); + argv_main = argv; qemu_init_subsystems();
Add a qemu_exec_requested() hook that causes the main loop to exit and re-exec qemu using the same initial arguments. If /usr/bin/qemu-exec exists, exec that instead. This is an optional site-specific trampoline that may alter the environment before exec'ing the qemu binary. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- include/sysemu/runstate.h | 1 + include/sysemu/sysemu.h | 1 + softmmu/globals.c | 1 + softmmu/runstate.c | 28 ++++++++++++++++++++++++++++ softmmu/vl.c | 1 + 5 files changed, 32 insertions(+)