vhost-user payload union restrictions ?

[Dr. David Alan Gilbert]

(Resend, remembering to add list)
Hi,
  I'm trying to understand what restrictions there are on the
payload that's part of VhostUserMsg; and am confused by
inconsistencies.

Lets start with this version:

subprojects/libvhost-user/libvhost-user.h :
typedef struct VhostUserMsg {
    int request;

#define VHOST_USER_VERSION_MASK     (0x3)
#define VHOST_USER_REPLY_MASK       (0x1 << 2)
#define VHOST_USER_NEED_REPLY_MASK  (0x1 << 3)
    uint32_t flags;
    uint32_t size; /* the following payload size */

    union {
#define VHOST_USER_VRING_IDX_MASK   (0xff)
#define VHOST_USER_VRING_NOFD_MASK  (0x1 << 8)
        uint64_t u64;
        struct vhost_vring_state state;
        struct vhost_vring_addr addr;
        VhostUserMemory memory;
        VhostUserMemRegMsg memreg;
        VhostUserLog log;
        VhostUserConfig config;
        VhostUserVringArea area;
        VhostUserInflight inflight;
    } payload;

    int fds[VHOST_MEMORY_BASELINE_NREGIONS];
    int fd_num;
    uint8_t *data;
} VU_PACKED VhostUserMsg;

note the 'fds' array after the payload but before
the end of the structure.

But then there's the version in:
hw/virtio/vhost-user.c
typedef union {
#define VHOST_USER_VRING_IDX_MASK   (0xff)
#define VHOST_USER_VRING_NOFD_MASK  (0x1<<8)
        uint64_t u64;
        struct vhost_vring_state state;
        struct vhost_vring_addr addr;
        VhostUserMemory memory;
        VhostUserMemRegMsg mem_reg;
        VhostUserLog log;
        struct vhost_iotlb_msg iotlb;
        VhostUserConfig config;
        VhostUserCryptoSession session;
        VhostUserVringArea area;
        VhostUserInflight inflight;
} VhostUserPayload;

typedef struct VhostUserMsg {
    VhostUserHeader hdr;
    VhostUserPayload payload;
} QEMU_PACKED VhostUserMsg;

which hasn't got the 'fds' section.
Yet they're both marked as 'packed'.

That's a bit unfortunate for two structures with the same name.

Am I right in thinking that the vhost-user.c version is sent over
the wire, while the libvhost-user.h one is really just an interface?

Is it safe for me to add a new, larger entry in the payload union
without breaking existing clients?

I ended up at this question after trying to add a variable length
entry to the union:

typedef struct {
    VhostUserFSSlaveMsg fs;
    VhostUserFSSlaveMsgEntry entries[VHOST_USER_FS_SLAVE_MAX_ENTRIES];
} QEMU_PACKED VhostUserFSSlaveMsgMax;

...
union ....
        VhostUserFSSlaveMsg fs;
        VhostUserFSSlaveMsgMax fs_max; /* Never actually used */
} VhostUserPayload;

and in the .h I had:
typedef struct {
    /* Generic flags for the overall message */
    uint32_t flags;
    /* Number of entries */
    uint16_t count;
    /* Spare */
    uint16_t align;

    VhostUserFSSlaveMsgEntry entries[];
} VhostUserFSSlaveMsg;

    union {
...
        VhostUserInflight inflight;
        VhostUserFSSlaveMsg fs;
    } payload;

which is apparently OK in the .c version, and gcc is happy with the same
in the libvhost-user.h version; but clang gets upset by the .h
version because it doesn't like the variable length structure
before the end of the struct - which I have sympathy for.

Dave

-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK