Re: [Qemu-devel] [RFC PATCH RDMA support v5: 05/12] core RDMA migration logic w/ new protocol

[Paolo Bonzini <pbonzini@redhat.com>]

Il 09/04/2013 05:04, mrhines@linux.vnet.ibm.com ha scritto:
> +void qemu_rdma_disable(void *opaque);
> +void qemu_rdma_cleanup(void *opaque);
> +int qemu_rdma_client_init(void *opaque, Error **errp,
> +            bool chunk_register_destination);
> +int qemu_rdma_connect(void *opaque, Error **errp);
> +void *qemu_rdma_data_init(const char *host_port, Error **errp);
> +int qemu_rdma_server_init(void *opaque, Error **errp);
> +int qemu_rdma_server_prepare(void *opaque, Error **errp);
> +int qemu_rdma_drain_cq(QEMUFile *f);
> +int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf, 
> +                            int64_t pos, int size);
> +int qemu_rdma_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size);
> +int qemu_rdma_close(void *opaque);
> +size_t save_rdma_page(QEMUFile *f, ram_addr_t block_offset, 
> +            ram_addr_t offset, int cont, size_t size, bool zero);
> +void *qemu_fopen_rdma(void *opaque, const char * mode);
> +int qemu_rdma_get_fd(void *opaque);
> +int qemu_rdma_accept(void *opaque);
> +void rdma_start_outgoing_migration(void *opaque, const char *host_port, Error **errp);
> +void rdma_start_incoming_migration(const char * host_port, Error **errp);
> +int qemu_rdma_handle_registrations(QEMUFile *f);
> +int qemu_rdma_finish_registrations(QEMUFile *f);

I think you have accumulated some dead code, for example qemu_rdma_disable.

Also, most of these functions can be static now.

> +#else /* !defined(CONFIG_RDMA) */
> +#define NOT_CONFIGURED() do { printf("WARN: RDMA is not configured\n"); } while(0)
> +#define qemu_rdma_cleanup(...) NOT_CONFIGURED()
> +#define qemu_rdma_data_init(...) NOT_CONFIGURED() 
> +#define rdma_start_outgoing_migration(...) NOT_CONFIGURED()
> +#define rdma_start_incoming_migration(...) NOT_CONFIGURED()
> +#define qemu_rdma_handle_registrations(...) 0
> +#define qemu_rdma_finish_registrations(...) 0
> +#define qemu_rdma_get_buffer NULL
> +#define qemu_rdma_put_buffer NULL
> +#define qemu_rdma_close NULL
> +#define qemu_fopen_rdma(...) NULL
> +#define qemu_rdma_client_init(...) -1 
> +#define qemu_rdma_client_connect(...) -1 
> +#define qemu_rdma_server_init(...) -1 
> +#define qemu_rdma_server_prepare(...) -1 
> +#define qemu_rdma_drain_cq(...) -1 
> +#define save_rdma_page(...) -ENOTSUP
> +
> +#endif /* CONFIG_RDMA */
> +

Please leave the prototypes even if CONFIG_RDMA is not defined.

This is because these symbols should not have any references when
CONFIG_RDMA is not defined.  The prototypes do not hurt.

You should almost be there.  The only functions that have some
references left in arch_init.c should be save_rdma_page,
qemu_rdma_drain_cq, qemu_rdma_finish_registrations.  Turn these into
QEMUFileOps, and there will be no undefined references even with
--disable-rdma.

But actually, once you get there, it could even make sense to merge
rdma.c and migration-rdma.c into a single file (migration-rdma.c; put
the former migration-rdma.c last so that you do not need forward
declarations, we tend to avoid them).  You can then eliminate the header
completely!

Paolo