Hi Marc-André On 07/27/2015 11:52 PM, Marc-André Lureau wrote:
Hi On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang@xxxxxxxxxx> wrote:Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx> --- src/qemu/qemu_conf.h | 3 + src/qemu/qemu_driver.c | 4 ++ src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 3f73929..61d3462 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -46,6 +46,7 @@ # include "virclosecallbacks.h" # include "virhostdev.h" # include "virfile.h" +# include "virshm.h" # ifdef CPU_SETSIZE /* Linux */ # define QEMUD_CPUMASK_LEN CPU_SETSIZE @@ -235,6 +236,8 @@ struct _virQEMUDriver { /* Immutable pointer. Unsafe APIs. XXX */ virHashTablePtr sharedDevices; + virShmObjectListPtr shmlist; + /* Immutable pointer, self-locking APIs */ virPortAllocatorPtr remotePorts; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9dbe635..282ab45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -778,6 +778,9 @@ qemuStateInitialize(bool privileged, if (qemuMigrationErrorInit(qemu_driver) < 0) goto error; + if (!(qemu_driver->shmlist = virShmObjectListGetDefault())) + goto error; + if (privileged) { char *channeldir; @@ -1087,6 +1090,7 @@ qemuStateCleanup(void) virObjectUnref(qemu_driver->config); virObjectUnref(qemu_driver->hostdevMgr); virHashFree(qemu_driver->sharedDevices); + virObjectUnref(qemu_driver->shmlist); virObjectUnref(qemu_driver->caps); virQEMUCapsCacheFree(qemu_driver->qemuCapsCache); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1c0c734..84b3b5e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4321,6 +4321,154 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, } +static int +qemuPrepareShmemDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainShmemDefPtr shmem) +{ + int ret = -1; + virShmObjectPtr tmp; + virShmObjectListPtr list = driver->shmlist; + bool othercreate = false; + char *path = NULL; + bool teardownlabel = false; + bool teardownshm = false; + int type, fd; + + virObjectLock(list); + + if ((tmp = virShmObjectFindByName(list, shmem->name))) { + if (shmem->size > tmp->size) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Shmem object %s is already exists and " + "size is smaller than require size"), + tmp->name); + goto cleanup; + } + + if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0) + goto cleanup; + + if (virShmObjectSaveState(tmp, list->stateDir) < 0) + goto cleanup; + + virObjectUnlock(list); + return 0; + } + + if (!shmem->server.enabled) { + if ((fd = virShmCreate(shmem->name, shmem->size, false, &othercreate, 0600)) < 0) + goto cleanup; + VIR_FORCE_CLOSE(fd); + + if ((ret = virShmBuildPath(shmem->name, &path)) == -1) { + ignore_value(virShmUnlink(shmem->name)); + goto cleanup; + } else if (ret == -2 && !othercreate) {What is ret == -2 ?
when ret = -2 this means we do not support virShmBuildPath in that platform (this could only happened on some other platform which support shm_open/shm_unlink ,just like solaris, freebsd), at that platform the shm obj is not in /dev/shm or not exist in the file system, if we help to create that shm obj but cannot find a way to relabel it, qemu will cannot use the shm in that case, so unlink the shm and let qemu create it will make guest can start success, and we could unlink the qemu create shm obj if there is no guest use it.
+ ignore_value(virShmUnlink(shmem->name)); + } + type = VIR_SHM_TYPE_SHM; + } else { + if (!virFileExists(shmem->server.chr.data.nix.path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Shmem device server socket is not exist"));is not / does not+ goto cleanup; + } else { + othercreate = true; + } + type = VIR_SHM_TYPE_SERVER; + } + teardownshm = true; + + if (virSecurityManagerSetShmemLabel(driver->securityManager, + vm->def, shmem, path) < 0) + goto cleanup;So each time a ivshmem device starts, it will potentially change the labels. Not sure that's a good idea. Why not apply only when created?
Good question, we allow specify the label in the shmem device XML, and if the user create the shm with a label which only allow that guest access, then the other guest will cannot use it if we do not change the label, but if we change the label to a label which allow all libvirt guest can access, it will make the first guest not safe.(Also this will happen when different guest use one shared disk), i have an quick thought to solve this issue:
Introduce the <shareable/> in the shmem device and forbid use one shm object in different shared policy (a example: two guest use one shm obj and one use it with a shared label and another not), and write some document to point out this.
Btw, have you considered managing shm like storage pools? Would that bring any benefit?
I thought about this before, and in my opinion introduce a lot of public API (just like:list, delete, create...) is unnecessary: there are two usage of shmem device: use it for a guest-host connect or for guest-guest connect. if one guest is using the shmem resource (ivshmem-server/shm obj), we should make sure we won't remove the resource. and after no guest use it we will clean up the resource. Also we provide a way to keep the resource here (we still will relabel the resource) after all guest is not use it, and users could clean the resource when they want. As the requirement for set up and clean up shmem device resource is very clear, we could help users to do this(the users can just use it without prepare extra code to find a way to know when need set up/clean up the resource).
Thanks a lot for your review and help. Luyao -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list