* Max Reitz (mre...@redhat.com) wrote: > On 29.06.20 17:41, Dr. David Alan Gilbert wrote: > > * Max Reitz (mre...@redhat.com) wrote: > >> Hi, > >> > >> In an iotest, I’m trying to quit qemu immediately after a migration has > >> failed. Unfortunately, that doesn’t seem to be possible in a clean way: > >> migrate_fd_cleanup() runs only at some point after the migration state > >> is already “failed”, so if I just wait for that “failed” state and > >> immediately quit, some cleanup functions may not have been run yet. > > > > Yeh this is hard; I always take the end of migrate_fd_cleanup to be the > > real end. > > Yes, unfortunately I don’t seem to have a way to look for that end. :( > > > It always happens on the main thread I think (it's done as a bh in some > > cases). > > > >> This is a problem with dirty bitmap migration at least, because it > >> increases the refcount on all block devices that are to be migrated, so > >> if we don’t call the cleanup function before quitting, the refcount will > >> stay elevated and bdrv_close_all() will hit an assertion because those > >> block devices are still around after blk_remove_all_bs() and > >> blockdev_close_all_bdrv_states(). > >> > >> In practice this particular issue might not be that big of a problem, > >> because it just means qemu aborts when the user intended to let it quit > >> anyway. But on one hand I could imagine that there are other clean-up > >> paths that should definitely run before qemu quits (although I don’t > >> know), and on the other, it’s a problem for my test. > > > > 'quit' varies - there are a lot of incoming failures that just assert; > > very few of them cause a clean exit (I think there are more clean ones > > after Peter's work on restartable postcopy a year or two ago). > > Well, my problem is about the source side, where there is still a VM > running that I would expect to be in a sane state even after a failed > migration.
Oh! Source side; the source side I worry much more about; if the destination implodes after a failed migration I'm not too worried - but the source side *shall not* fail. > > I do see the end of migrate_fd_cleanup calls the notifier list; but it's > > not clear to me that it's alwyas going to see the first transition to > > 'failed' at that point. > > What exactly do you mean? It appears to me that both query-status and > the MIGRATION events signal the failed state before migrate_fd_cleanup() > is invoked. OK, I was getting confused between event sending and the notifiers; the event happens a bit before. The state gets set to failed and then we call fd_cleanup in most cases; the state is what query-status is keyed off, and is also what causes the event to be sent. > If you mean I could add a notifier to that list to do something™, I’m > not sure what exactly it is I’d so. Yeh, that's why the notifiers are there.... However, if you need to do some cleanup at the end of a migration, then I think adding a call inside migrate_fd_cleanup is perfectly fine. > My test can’t do it, because it’s > an iotest, and even if it could, I suppose I’d want to wait until even > after all notifiers have been invoked (which isn’t guaranteed if I’d add > a notifier myself). > > >> I tried working around the problem for my test by waiting on “Unable to > >> write” appearing on stderr, because that indicates that > >> migrate_fd_cleanup()’s error_report_err() has been reached. But on one > >> hand, that isn’t really nice, and on the other, it doesn’t even work > >> when the failure is on the source side (because then there is no > >> s->error for migrate_fd_cleanup() to report). > >> > >> In all, I’m asking: > >> (1) Is there a nice solution for me now to delay quitting qemu until the > >> failed migration has been fully resolved, including the clean-up? > > > > In vl.c, I added a call to migration_shutdown in qemu_cleanup - although > > that seems to be mostly about cleaning up the *outgoing* side; you could > > add some incoming cleanup there. > > So you mean waiting until migrate_fd_cleanup() has run? Maybe I’ll try > that tomorrow, although I’d hoped I could get this done without having > to modify the code base... (I.e., I’d hoped there would be some > QMP-queriable flag somewhere that could tell me whether the > migrate_fd_cleanup() has run) Please be a little careful around there; i.e. try as far as possible not to hang on dead block devices. > >> (2) Isn’t it a problem if qemu crashes when you issue “quit” via QMP at > >> the wrong time? Like, maybe lingering subprocesses when using “exec”? > > > > Yeh that should be cleaner, but isn't. > > :( Randomly quitting is safer than it used to be (again that qemu_cleanup code is what should make it so). Dave > OK then. Thanks for your insights! > > Max > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
