Re: [Qemu-devel] [PATCH 1/2] virtio-9p: Add support for 9p migration.

[Boris Sukholitko]

On Tue, Sep 30, 2014 at 12:48 AM, Benoît Canet <address@hidden> wrote:
> The Friday 26 Sep 2014 à 18:19:56 (+0300), Boris Sukholitko wrote :
>
>
>> This patch is a rebase of Aneesh Kumar's and Benoit Canet's previous
>> work.
>>
>> Signed-off-by: Boris Sukholitko <address@hidden>
>
> If Aneesh and me worked on this you should also keep our signed-off in 
> addition
> of yours.
>

I will resend the patches with your Signed-off.

Aneesh, should I add your Signed-off as well?

Thanks,
Boris.


> Best regards
>
> Benoît
>
>> ---
>>  hw/9pfs/virtio-9p-device.c | 152 
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 152 insertions(+)
>>
>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>> index 2572747..078ad39 100644
>> --- a/hw/9pfs/virtio-9p-device.c
>> +++ b/hw/9pfs/virtio-9p-device.c
>> @@ -21,6 +21,149 @@
>>  #include "virtio-9p-coth.h"
>>  #include "hw/virtio/virtio-access.h"
>>
>> +static void virtio_9p_save_path(QEMUFile *f, V9fsPath *path)
>> +{
>> +    qemu_put_be16(f, path->size);
>> +    qemu_put_buffer(f, (const uint8_t *) path->data, path->size);
>> +}
>> +
>> +static void virtio_9p_save_string(QEMUFile *f, V9fsString *s)
>> +{
>> +    qemu_put_be16(f, s->size);
>> +    qemu_put_buffer(f, (const uint8_t *) s->data, s->size);
>> +}
>> +
>> +static void virtio_9p_save_xattr(QEMUFile *f, V9fsXattr *xattr)
>> +{
>> +    qemu_put_be64(f, xattr->copied_len);
>> +    qemu_put_be64(f, xattr->len);
>> +    qemu_put_buffer(f, (const uint8_t *) xattr->value, xattr->len);
>> +    virtio_9p_save_string(f, &xattr->name);
>> +    qemu_put_be32(f, xattr->flags);
>> +}
>> +
>> +static void virtio_9p_save_fid(QEMUFile *f, V9fsFidState *fid)
>> +{
>> +    /* First close the fid and mark it for reopen if migration fail */
>> +    if (fid->fid_type == P9_FID_FILE) {
>> +        close(fid->fs.fd);
>> +        fid->fs.fd = -1;
>> +    } else if (fid->fid_type == P9_FID_DIR) {
>> +        closedir(fid->fs.dir);
>> +        fid->fs.dir = NULL;
>> +    }
>> +
>> +    qemu_put_be32(f, fid->fid_type);
>> +    if (fid->fid_type == P9_FID_XATTR) {
>> +        /* we don't save fs_reclaim */
>> +        virtio_9p_save_xattr(f, &fid->fs.xattr);
>> +    }
>> +    qemu_put_be32(f, fid->fid);
>> +    virtio_9p_save_path(f, &fid->path);
>> +    qemu_put_be32(f, fid->flags);
>> +    qemu_put_be32(f, fid->open_flags);
>> +    qemu_put_be32(f, fid->uid);
>> +    qemu_put_be32(f, fid->ref);
>> +    qemu_put_be32(f, fid->clunked);
>> +}
>> +
>> +static void virtio_9p_save(QEMUFile *f, void *opaque)
>> +{
>> +    int fidcount = 0;
>> +    V9fsState *s = opaque;
>> +    V9fsFidState *fid;
>> +
>> +    virtio_save(VIRTIO_DEVICE(s), f);
>> +
>> +    for (fid = s->fid_list; fid; fid = fid->next) {
>> +        fidcount++;
>> +    }
>> +    /* Write the total number of fid structure */
>> +    qemu_put_be32(f, fidcount);
>> +
>> +    for (fid = s->fid_list; fid; fid = fid->next) {
>> +        virtio_9p_save_fid(f, fid);
>> +    }
>> +
>> +    qemu_put_be32(f, s->proto_version);
>> +    qemu_put_be32(f, s->msize);
>> +}
>> +
>> +static void virtio_9p_load_path(QEMUFile *f, V9fsPath *path)
>> +{
>> +    path->size = qemu_get_be16(f);
>> +    path->data = g_malloc0(path->size);
>> +    qemu_get_buffer(f, (uint8_t *) path->data, path->size);
>> +}
>> +
>> +static void virtio_9p_load_string(QEMUFile *f, V9fsString *s)
>> +{
>> +    s->size = qemu_get_be16(f);
>> +    s->data = g_malloc0(s->size);
>> +    qemu_get_buffer(f, (uint8_t *) s->data, s->size);
>> +}
>> +
>> +static void virtio_9p_load_xattr(QEMUFile *f, V9fsXattr *xattr)
>> +{
>> +    xattr->copied_len = qemu_get_be64(f);
>> +    xattr->len = qemu_get_be64(f);
>> +    qemu_get_buffer(f, (uint8_t *) xattr->value, xattr->len);
>> +    virtio_9p_load_string(f, &xattr->name);
>> +    xattr->flags = qemu_get_be32(f);
>> +}
>> +
>> +static V9fsFidState *virtio_9p_load_fid(QEMUFile *f)
>> +{
>> +    V9fsFidState *fid;
>> +    fid = g_new0(V9fsFidState, 1);
>> +
>> +    fid->fid_type    = qemu_get_be32(f);
>> +    if (fid->fid_type == P9_FID_XATTR) {
>> +        virtio_9p_load_xattr(f, &fid->fs.xattr);
>> +    }
>> +    fid->fid         = qemu_get_be32(f);
>> +    virtio_9p_load_path(f, &fid->path);
>> +    fid->flags       = qemu_get_be32(f);
>> +    fid->open_flags  = qemu_get_be32(f);
>> +    fid->uid         = qemu_get_be32(f);
>> +    fid->ref         = qemu_get_be32(f);
>> +    fid->clunked     = qemu_get_be32(f);
>> +
>> +    /* If it's a file fid mark the file descriptors as closed.
>> +     * DIR is null thanks to g_new0.
>> +     * When doing get_fid v9fs_reopen_fid will reopen the file or the 
>> directory.
>> +     */
>> +    if (fid->fid_type == P9_FID_FILE) {
>> +        fid->fs.fd = -1;
>> +    }
>> +    return fid;
>> +}
>> +
>> +static int virtio_9p_load(QEMUFile *f, void *opaque, int version_id)
>> +{
>> +    int fidcount;
>> +    V9fsState *s = opaque;
>> +    V9fsFidState **fid;
>> +
>> +    if (version_id != 2) {
>> +        return -EINVAL;
>> +    }
>> +    virtio_load(VIRTIO_DEVICE(s), f, version_id);
>> +    fidcount = qemu_get_be32(f);
>> +
>> +    fid = &s->fid_list;
>> +    while (fidcount) {
>> +        *fid = virtio_9p_load_fid(f);
>> +        fid = &((*fid)->next);
>> +        fidcount--;
>> +    }
>> +
>> +    s->proto_version  = qemu_get_be32(f);
>> +    s->msize          = qemu_get_be32(f);
>> +
>> +    return 0;
>> +}
>> +
>>  static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t 
>> features)
>>  {
>>      features |= 1 << VIRTIO_9P_MOUNT_TAG;
>> @@ -50,6 +193,7 @@ static void virtio_9p_device_realize(DeviceState *dev, 
>> Error **errp)
>>      struct stat stat;
>>      FsDriverEntry *fse;
>>      V9fsPath path;
>> +    static int virtio_9p_id;
>>
>>      virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P,
>>                  sizeof(struct virtio_9p_config) + MAX_TAG_LEN);
>> @@ -98,6 +242,14 @@ static void virtio_9p_device_realize(DeviceState *dev, 
>> Error **errp)
>>      s->ops = fse->ops;
>>      s->config_size = sizeof(struct virtio_9p_config) + len;
>>      s->fid_list = NULL;
>> +
>> +    /* Original patch says that instance id must be derived of tag
>> +     * name. Hash functions do have collisions so why would it be better
>> +     * than an increment ?
>> +     */
>> +    register_savevm(dev, "virtio-9p", virtio_9p_id++, 2,
>> +                    virtio_9p_save, virtio_9p_load, s);
>> +
>>      qemu_co_rwlock_init(&s->rename_lock);
>>
>>      if (s->ops->init(&s->ctx) < 0) {
>> --
>> 2.0.2
>>
>>