1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH 0/1] add exit-script option to qemu

Dominik Csapak posted 1 patch 5 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181003091344.24496-1-d.csapak@proxmox.com
Test docker-clang@ubuntu failed
Test checkpatch passed
There is a newer version of this series
qemu-options.hx | 18 ++++++++++++++++++
vl.c            | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+)
[Qemu-devel] [PATCH 0/1] add exit-script option to qemu
Posted by Dominik Csapak 5 years, 6 months ago 
this patch aims to execute a script when qemu exits
so that one can do cleanups when using --daemonize without
having to use the qmp monitor

for now i have mostly copied the script execution code from
the launch_script function of net/tap.c as i am not sure
if it would make sense to refactor that and if it does, where
to put such a function

Dominik Csapak (1):
  vl.c: call optional script when exiting

 qemu-options.hx | 18 ++++++++++++++++++
 vl.c            | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

-- 
2.11.0
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago 
Hi Dominik,

On 03/10/2018 11:13, Dominik Csapak wrote:
> this patch aims to execute a script when qemu exits
> so that one can do cleanups when using --daemonize without
> having to use the qmp monitor
> 
> for now i have mostly copied the script execution code from
> the launch_script function of net/tap.c as i am not sure
> if it would make sense to refactor that and if it does, where
> to put such a function

I'd rather refactor a 'more of 10 common LOC function'.

Maybe qemu_launch_script() in "qemu/osdep.h"?

Phil.
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
Posted by Daniel P. Berrangé 5 years, 6 months ago 
On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote:
> this patch aims to execute a script when qemu exits
> so that one can do cleanups when using --daemonize without
> having to use the qmp monitor

IMHO the idea of cleanup scripts run by QEMU itself is flawed.
QEMU will inevitably crash before cleanup scripts can be run,
so whatever mgmt app is using QEMU needs to be able to do
cleanup without QEMU's help.

I think this can be done more reliably with a wrapper script,
that spawns QEMU, waits for it to exit and then calls the
cleanup script. On Linux at least you can use prctl() with
PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU
even after it has daemonized.

Perhaps we could have such a wrapper script put in the
contrib directory

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
Posted by Dominik Csapak 5 years, 6 months ago 
On 10/4/18 3:51 PM, Daniel P. Berrangé wrote:
> On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote:
>> this patch aims to execute a script when qemu exits
>> so that one can do cleanups when using --daemonize without
>> having to use the qmp monitor
> 
> IMHO the idea of cleanup scripts run by QEMU itself is flawed.
> QEMU will inevitably crash before cleanup scripts can be run,
> so whatever mgmt app is using QEMU needs to be able to do
> cleanup without QEMU's help.
> 
> I think this can be done more reliably with a wrapper script,
> that spawns QEMU, waits for it to exit and then calls the
> cleanup script. On Linux at least you can use prctl() with
> PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU
> even after it has daemonized.
> 
> Perhaps we could have such a wrapper script put in the
> contrib directory
> 
> Regards,
> Daniel
> 
Hi,

for cleaning up after qemu crashes, you are completely right,
(ignoring that the downscript for tap devices also never gets executed
then), but this series has another use.

With it, a user can determine the reason of a graceful shutdown
(e.g., if it was by a signal, qmp or from inside) of qemu,
especially when using -no-reboot without using qmp

and using qmp for that is not very practical for everyone,
or is there another way for that which i am missing?

Regards,
Dominik
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
Posted by Daniel P. Berrangé 5 years, 6 months ago 
On Fri, Oct 05, 2018 at 08:56:27AM +0200, Dominik Csapak wrote:
> On 10/4/18 3:51 PM, Daniel P. Berrangé wrote:
> > On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote:
> > > this patch aims to execute a script when qemu exits
> > > so that one can do cleanups when using --daemonize without
> > > having to use the qmp monitor
> > 
> > IMHO the idea of cleanup scripts run by QEMU itself is flawed.
> > QEMU will inevitably crash before cleanup scripts can be run,
> > so whatever mgmt app is using QEMU needs to be able to do
> > cleanup without QEMU's help.
> > 
> > I think this can be done more reliably with a wrapper script,
> > that spawns QEMU, waits for it to exit and then calls the
> > cleanup script. On Linux at least you can use prctl() with
> > PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU
> > even after it has daemonized.
> > 
> > Perhaps we could have such a wrapper script put in the
> > contrib directory
> > 
> > Regards,
> > Daniel
> > 
> Hi,
> 
> for cleaning up after qemu crashes, you are completely right,
> (ignoring that the downscript for tap devices also never gets executed
> then), but this series has another use.
> 
> With it, a user can determine the reason of a graceful shutdown
> (e.g., if it was by a signal, qmp or from inside) of qemu,
> especially when using -no-reboot without using qmp
> 
> and using qmp for that is not very practical for everyone,
> or is there another way for that which i am missing?

Honestly QMP *is* the right answer. We've put alot of effort into QMP
and I don't think it is sensible to start adding new mechanisms to
provide the same information in an adhoc manner.

What makes you think QMP isn't practical to use ?  We have client
impls that talk to QMP in scripts/qmp that are just a few 100 lines
of pretty simple python code.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
Posted by Dominik Csapak 5 years, 6 months ago 
On 10/5/18 10:38 AM, Daniel P. Berrangé wrote:
> On Fri, Oct 05, 2018 at 08:56:27AM +0200, Dominik Csapak wrote:
>> On 10/4/18 3:51 PM, Daniel P. Berrangé wrote:
>>> On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote:
>>>> this patch aims to execute a script when qemu exits
>>>> so that one can do cleanups when using --daemonize without
>>>> having to use the qmp monitor
>>>
>>> IMHO the idea of cleanup scripts run by QEMU itself is flawed.
>>> QEMU will inevitably crash before cleanup scripts can be run,
>>> so whatever mgmt app is using QEMU needs to be able to do
>>> cleanup without QEMU's help.
>>>
>>> I think this can be done more reliably with a wrapper script,
>>> that spawns QEMU, waits for it to exit and then calls the
>>> cleanup script. On Linux at least you can use prctl() with
>>> PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU
>>> even after it has daemonized.
>>>
>>> Perhaps we could have such a wrapper script put in the
>>> contrib directory
>>>
>>> Regards,
>>> Daniel
>>>
>> Hi,
>>
>> for cleaning up after qemu crashes, you are completely right,
>> (ignoring that the downscript for tap devices also never gets executed
>> then), but this series has another use.
>>
>> With it, a user can determine the reason of a graceful shutdown
>> (e.g., if it was by a signal, qmp or from inside) of qemu,
>> especially when using -no-reboot without using qmp
>>
>> and using qmp for that is not very practical for everyone,
>> or is there another way for that which i am missing?
> 
> Honestly QMP *is* the right answer. We've put alot of effort into QMP
> and I don't think it is sensible to start adding new mechanisms to
> provide the same information in an adhoc manner.
> 
> What makes you think QMP isn't practical to use ?  We have client
> impls that talk to QMP in scripts/qmp that are just a few 100 lines
> of pretty simple python code.
> 

ok, i just found that having to start an extra program waiting for qmp
events might be overkill for some users

nonetheless, i just found out that even with qmp, there is no way
to see if a machine started with '-no-reboot' was trying to reboot
or just shutting down, in both cases i got a SHUTDOWN event
with 'guest: true'

would it make sense to send a patch that introduces a new data field
for the shutdown event that says if it was really a reset?
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
Posted by Daniel P. Berrangé 5 years, 6 months ago 
On Fri, Oct 05, 2018 at 02:57:03PM +0200, Dominik Csapak wrote:
> On 10/5/18 10:38 AM, Daniel P. Berrangé wrote:
> > On Fri, Oct 05, 2018 at 08:56:27AM +0200, Dominik Csapak wrote:
> > > On 10/4/18 3:51 PM, Daniel P. Berrangé wrote:
> > > > On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote:
> > > > > this patch aims to execute a script when qemu exits
> > > > > so that one can do cleanups when using --daemonize without
> > > > > having to use the qmp monitor
> > > > 
> > > > IMHO the idea of cleanup scripts run by QEMU itself is flawed.
> > > > QEMU will inevitably crash before cleanup scripts can be run,
> > > > so whatever mgmt app is using QEMU needs to be able to do
> > > > cleanup without QEMU's help.
> > > > 
> > > > I think this can be done more reliably with a wrapper script,
> > > > that spawns QEMU, waits for it to exit and then calls the
> > > > cleanup script. On Linux at least you can use prctl() with
> > > > PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU
> > > > even after it has daemonized.
> > > > 
> > > > Perhaps we could have such a wrapper script put in the
> > > > contrib directory
> > > > 
> > > > Regards,
> > > > Daniel
> > > > 
> > > Hi,
> > > 
> > > for cleaning up after qemu crashes, you are completely right,
> > > (ignoring that the downscript for tap devices also never gets executed
> > > then), but this series has another use.
> > > 
> > > With it, a user can determine the reason of a graceful shutdown
> > > (e.g., if it was by a signal, qmp or from inside) of qemu,
> > > especially when using -no-reboot without using qmp
> > > 
> > > and using qmp for that is not very practical for everyone,
> > > or is there another way for that which i am missing?
> > 
> > Honestly QMP *is* the right answer. We've put alot of effort into QMP
> > and I don't think it is sensible to start adding new mechanisms to
> > provide the same information in an adhoc manner.
> > 
> > What makes you think QMP isn't practical to use ?  We have client
> > impls that talk to QMP in scripts/qmp that are just a few 100 lines
> > of pretty simple python code.
> > 
> 
> ok, i just found that having to start an extra program waiting for qmp
> events might be overkill for some users
> 
> nonetheless, i just found out that even with qmp, there is no way
> to see if a machine started with '-no-reboot' was trying to reboot
> or just shutting down, in both cases i got a SHUTDOWN event
> with 'guest: true'
> 
> would it make sense to send a patch that introduces a new data field
> for the shutdown event that says if it was really a reset?

I had a feeling there was another way to detct it, but I'm not seeing
it in QMP. So yeah, if this can't be determined, then I expect it is
worth extending QMP to report this 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.