[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/4] Introduce yank feature
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v2 1/4] Introduce yank feature |
Date: |
Thu, 21 May 2020 16:48:06 +0100 |
User-agent: |
Mutt/1.13.4 (2020-02-15) |
On Thu, May 21, 2020 at 05:42:41PM +0200, Lukas Straub wrote:
> On Thu, 21 May 2020 16:03:35 +0100
> Stefan Hajnoczi <address@hidden> wrote:
>
> > On Wed, May 20, 2020 at 11:05:39PM +0200, Lukas Straub wrote:
> > > +void yank_generic_iochannel(void *opaque)
> > > +{
> > > + QIOChannel *ioc = QIO_CHANNEL(opaque);
> > > +
> > > + qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> > > +}
> > > +
> > > +void qmp_yank(strList *instances, Error **errp)
> > > +{
> > > + strList *tmp;
> > > + struct YankInstance *instance;
> > > + struct YankFuncAndParam *entry;
> > > +
> > > + qemu_mutex_lock(&lock);
> > > + tmp = instances;
> > > + for (; tmp; tmp = tmp->next) {
> > > + instance = yank_find_instance(tmp->value);
> > > + if (!instance) {
> > > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> > > + "Instance '%s' not found", tmp->value);
> > > + qemu_mutex_unlock(&lock);
> > > + return;
> > > + }
> > > + }
> > > + tmp = instances;
> > > + for (; tmp; tmp = tmp->next) {
> > > + instance = yank_find_instance(tmp->value);
> > > + assert(instance);
> > > + QLIST_FOREACH(entry, &instance->yankfns, next) {
> > > + entry->func(entry->opaque);
> > > + }
> > > + }
> > > + qemu_mutex_unlock(&lock);
> > > +}
> >
> > From docs/devel/qapi-code-gen.txt:
> >
> > An OOB-capable command handler must satisfy the following conditions:
> >
> > - It terminates quickly.
> Check.
>
> > - It does not invoke system calls that may block.
> brk/sbrk (malloc and friends):
> The manpage doesn't say anything about blocking, but malloc is already used
> while handling the qmp command.
>
> shutdown():
> The manpage doesn't say anything about blocking, but this is already used in
> migration oob qmp commands.
It just marks the socket state in local kernel side. It doesn't involve
any blocking roundtrips over the wire, so this is fine.
>
> There are no other syscalls involved to my knowledge.
>
> > - It does not access guest RAM that may block when userfaultfd is
> > enabled for postcopy live migration.
> Check.
>
> > - It takes only "fast" locks, i.e. all critical sections protected by
> > any lock it takes also satisfy the conditions for OOB command
> > handler code.
>
> The lock in yank.c satisfies this requirement.
>
> qio_channel_shutdown doesn't take any locks.
Agreed, I think the yank code is compliant with all the points
listed above.
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 :|
- [PATCH v2 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu, Lukas Straub, 2020/05/20
- [PATCH v2 2/4] block/nbd.c: Add yank feature, Lukas Straub, 2020/05/20
- [PATCH v2 3/4] chardev/char-socket.c: Add yank feature, Lukas Straub, 2020/05/20
- [PATCH v2 4/4] migration: Add yank feature, Lukas Straub, 2020/05/20
- Re: [PATCH v2 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu, no-reply, 2020/05/20
- Re: [PATCH v2 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu, no-reply, 2020/05/20