[libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device

[lhuang]

On 07/30/2015 06:25 PM, Daniel P. Berrange wrote:
> On Thu, Jul 23, 2015 at 06:13:49PM +0800, Luyao Huang wrote:
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>>   src/qemu/qemu_conf.h    |   3 +
>>   src/qemu/qemu_driver.c  |   4 ++
>>   src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 165 insertions(+)
>>
>> +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) {
>> +            ignore_value(virShmUnlink(shmem->name));
> Why are you treating -1 differentl from -2 - in both cases we should
> abort creation as that indicates the method either failed or is not
> supported in this platform.

What i thought when i wrote this is : 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) but we could use shm_open, on 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.

I am not sure this is a good idea right now, since i am not sure this 
will work as except on different platform. Maybe i should remove it and 
make virShmBuildPath return -1 if not support on that platform.

> Regards,
> Daniel

Luyao