On Thu, 21 May 2020 16:03:35 +0100 Stefan Hajnoczi <stefa...@gmail.com> 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.
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.
Regards,
Lukas Straub
> This patch series violates these rules and calls existing functions that
> were not designed for OOB execution.
>
> Please explain why it is safe to do this.
>
> Stefan
pgp_1TlQz3010.pgp
Description: OpenPGP digital signature
