Commit 47451466 authored by Peter Xu's avatar Peter Xu Committed by Markus Armbruster
Browse files

monitor: add lock to protect mon_fdsets 



Introduce a new global big lock for mon_fdsets.  Take it where needed.

The monitor_fdset_get_fd() handling is a bit tricky: now we need to call
qemu_mutex_unlock() which might pollute errno, so we need to make sure
the correct errno be passed up to the callers.  To make things simpler,
we let monitor_fdset_get_fd() return the -errno directly when error
happens, then in qemu_open() we move it back into errno.

Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarPeter Xu <peterx@redhat.com>
Message-Id: <20180608035511.7439-8-peterx@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
parent d32749de

monitor.c

+42 −10
Original line number Diff line number Diff line
@@ -271,7 +271,10 @@ static QemuMutex monitor_lock; 
static GHashTable *monitor_qapi_event_state;

static QTAILQ_HEAD(mon_list, Monitor) mon_list;



/* Protects mon_fdsets */

static QemuMutex mon_fdsets_lock;

static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets;



static int mon_refcount;



static mon_cmd_t mon_cmds[];
@@ -2319,9 +2322,11 @@ static void monitor_fdsets_cleanup(void) 
    MonFdset *mon_fdset;

    MonFdset *mon_fdset_next;



    qemu_mutex_lock(&mon_fdsets_lock);

    QLIST_FOREACH_SAFE(mon_fdset, &mon_fdsets, next, mon_fdset_next) {

        monitor_fdset_cleanup(mon_fdset);

    }

    qemu_mutex_unlock(&mon_fdsets_lock);

}



AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
@@ -2356,6 +2361,7 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) 
    MonFdsetFd *mon_fdset_fd;

    char fd_str[60];



    qemu_mutex_lock(&mon_fdsets_lock);

    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {

        if (mon_fdset->id != fdset_id) {

            continue;
@@ -2375,10 +2381,12 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) 
            goto error;

        }

        monitor_fdset_cleanup(mon_fdset);

        qemu_mutex_unlock(&mon_fdsets_lock);

        return;

    }



error:

    qemu_mutex_unlock(&mon_fdsets_lock);

    if (has_fd) {

        snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64,

                 fdset_id, fd);
@@ -2394,6 +2402,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) 
    MonFdsetFd *mon_fdset_fd;

    FdsetInfoList *fdset_list = NULL;



    qemu_mutex_lock(&mon_fdsets_lock);

    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {

        FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info));

        FdsetFdInfoList *fdsetfd_list = NULL;
@@ -2423,6 +2432,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) 
        fdset_info->next = fdset_list;

        fdset_list = fdset_info;

    }

    qemu_mutex_unlock(&mon_fdsets_lock);



    return fdset_list;

}
@@ -2435,6 +2445,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id, 
    MonFdsetFd *mon_fdset_fd;

    AddfdInfo *fdinfo;



    qemu_mutex_lock(&mon_fdsets_lock);

    if (has_fdset_id) {

        QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {

            /* Break if match found or match impossible due to ordering by ID */
@@ -2455,6 +2466,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id, 
            if (fdset_id < 0) {

                error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "fdset-id",

                           "a non-negative value");

                qemu_mutex_unlock(&mon_fdsets_lock);

                return NULL;

            }

            /* Use specified fdset ID */
@@ -2505,16 +2517,21 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id, 
    fdinfo->fdset_id = mon_fdset->id;

    fdinfo->fd = mon_fdset_fd->fd;



    qemu_mutex_unlock(&mon_fdsets_lock);

    return fdinfo;

}



int monitor_fdset_get_fd(int64_t fdset_id, int flags)

{

#ifndef _WIN32

#ifdef _WIN32

    return -ENOENT;

#else

    MonFdset *mon_fdset;

    MonFdsetFd *mon_fdset_fd;

    int mon_fd_flags;

    int ret;



    qemu_mutex_lock(&mon_fdsets_lock);

    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {

        if (mon_fdset->id != fdset_id) {

            continue;
@@ -2522,20 +2539,24 @@ int monitor_fdset_get_fd(int64_t fdset_id, int flags) 
        QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {

            mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL);

            if (mon_fd_flags == -1) {

                return -1;

                ret = -errno;

                goto out;

            }



            if ((flags & O_ACCMODE) == (mon_fd_flags & O_ACCMODE)) {

                return mon_fdset_fd->fd;

                ret = mon_fdset_fd->fd;

                goto out;

            }

        }

        errno = EACCES;

        return -1;

        ret = -EACCES;

        goto out;

    }

#endif

    ret = -ENOENT;



    errno = ENOENT;

    return -1;

out:

    qemu_mutex_unlock(&mon_fdsets_lock);

    return ret;

#endif

}



int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
@@ -2543,20 +2564,25 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) 
    MonFdset *mon_fdset;

    MonFdsetFd *mon_fdset_fd_dup;



    qemu_mutex_lock(&mon_fdsets_lock);

    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {

        if (mon_fdset->id != fdset_id) {

            continue;

        }

        QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {

            if (mon_fdset_fd_dup->fd == dup_fd) {

                return -1;

                goto err;

            }

        }

        mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));

        mon_fdset_fd_dup->fd = dup_fd;

        QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next);

        qemu_mutex_unlock(&mon_fdsets_lock);

        return 0;

    }



err:

    qemu_mutex_unlock(&mon_fdsets_lock);

    return -1;

}
@@ -2565,6 +2591,7 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove) 
    MonFdset *mon_fdset;

    MonFdsetFd *mon_fdset_fd_dup;



    qemu_mutex_lock(&mon_fdsets_lock);

    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {

        QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {

            if (mon_fdset_fd_dup->fd == dup_fd) {
@@ -2573,13 +2600,17 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove) 
                    if (QLIST_EMPTY(&mon_fdset->dup_fds)) {

                        monitor_fdset_cleanup(mon_fdset);

                    }

                    return -1;

                    goto err;

                } else {

                    qemu_mutex_unlock(&mon_fdsets_lock);

                    return mon_fdset->id;

                }

            }

        }

    }



err:

    qemu_mutex_unlock(&mon_fdsets_lock);

    return -1;

}
@@ -4519,6 +4550,7 @@ void monitor_init_globals(void) 
    monitor_qapi_event_init();

    sortcmdlist();

    qemu_mutex_init(&monitor_lock);

    qemu_mutex_init(&mon_fdsets_lock);

    monitor_iothread_init();

}

stubs/fdset.c

+1 −1
Original line number Diff line number Diff line
@@ -14,7 +14,7 @@ int monitor_fdset_dup_fd_find(int dup_fd) 


int monitor_fdset_get_fd(int64_t fdset_id, int flags)

{

    return -1;

    return -ENOENT;

}



void monitor_fdset_dup_fd_remove(int dupfd)

util/osdep.c

+2 −1
Original line number Diff line number Diff line
@@ -302,7 +302,8 @@ int qemu_open(const char *name, int flags, ...) 
        }



        fd = monitor_fdset_get_fd(fdset_id, flags);

        if (fd == -1) {

        if (fd < 0) {

            errno = -fd;

            return -1;

        }