From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v4 3/3] libxl_qmp: remove libxl__qmp_migrate, introduce libxl__qmp_save Date: Thu, 2 Feb 2012 14:41:40 +0000 Message-ID: <1328193700.2924.15.camel@zakaz.uk.xensource.com> References: <1328119839-1168-3-git-send-email-stefano.stabellini@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1328119839-1168-3-git-send-email-stefano.stabellini@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Stefano Stabellini Cc: "rshriram@cs.ubc.ca" , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On Wed, 2012-02-01 at 18:10 +0000, Stefano Stabellini wrote: > Following the recent changes to upstream Qemu, the best monitor command > to suit or needs is "save_devices" rather than "migrate". > This patch removes libxl__qmp_migrate and introduces libxl__qmp_save > instead, that uses "save_devices". > > Signed-off-by: Stefano Stabellini > --- > tools/libxl/libxl_dom.c | 11 +----- > tools/libxl/libxl_internal.h | 2 +- > tools/libxl/libxl_qmp.c | 82 ++---------------------------------------- > 3 files changed, 5 insertions(+), 90 deletions(-) > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index a6eb714..40ebcd1 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -771,18 +771,9 @@ int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd) > break; > } > case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN: > - fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR); > - if (fd2 < 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > - "Unable to create a QEMU save file\n"); > - return ERROR_FAIL; > - } > - /* Save DM state into fd2 */ > - ret = libxl__qmp_migrate(gc, domid, fd2); > + ret = libxl__qmp_save(gc, domid, (char *)filename); I don't think you need this cast, your qmp_save takes a const char *. Otherwise this all looks alright to me. Ian. > if (ret) > goto out; > - close(fd2); > - fd2 = -1; > break; > default: > return ERROR_INVAL; > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 3c8da45..6d11cfe 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -619,7 +619,7 @@ _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev); > _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid, > libxl_device_pci *pcidev); > /* Save current QEMU state into fd. */ > -_hidden int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd); > +_hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename); > /* close and free the QMP handler */ > _hidden void libxl__qmp_close(libxl__qmp_handler *qmp); > /* remove the socket file, if the file has already been removed, > diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c > index 6d401b7..19bf699 100644 > --- a/tools/libxl/libxl_qmp.c > +++ b/tools/libxl/libxl_qmp.c > @@ -533,52 +533,6 @@ out: > return rc; > } > > -static int qmp_send_fd(libxl__gc *gc, libxl__qmp_handler *qmp, > - libxl_key_value_list *args, > - qmp_callback_t callback, void *opaque, > - qmp_request_context *context, > - int fd) > -{ > - struct msghdr msg = { 0 }; > - struct cmsghdr *cmsg; > - char control[CMSG_SPACE(sizeof (fd))]; > - struct iovec iov; > - char *buf = NULL; > - > - buf = qmp_send_prepare(gc, qmp, "getfd", args, callback, opaque, context); > - > - /* Response data */ > - iov.iov_base = buf; > - iov.iov_len = strlen(buf); > - > - /* compose the message */ > - msg.msg_iov = &iov; > - msg.msg_iovlen = 1; > - msg.msg_control = control; > - msg.msg_controllen = sizeof (control); > - > - /* attach open fd */ > - cmsg = CMSG_FIRSTHDR(&msg); > - cmsg->cmsg_level = SOL_SOCKET; > - cmsg->cmsg_type = SCM_RIGHTS; > - cmsg->cmsg_len = CMSG_LEN(sizeof (fd)); > - *(int *)CMSG_DATA(cmsg) = fd; > - > - msg.msg_controllen = cmsg->cmsg_len; > - > - if (sendmsg(qmp->qmp_fd, &msg, 0) < 0) { > - LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, > - "Failed to send a QMP message to QEMU."); > - return ERROR_FAIL; > - } > - if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, "\r\n", 2, > - "CRLF", "QMP socket")) { > - return ERROR_FAIL; > - } > - > - return qmp->last_id_used; > -} > - > static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd, > libxl_key_value_list *args, > qmp_callback_t callback, void *opaque, > @@ -815,34 +769,8 @@ int libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci *pcidev) > return qmp_device_del(gc, domid, id); > } > > -static int qmp_getfd(libxl__gc *gc, libxl__qmp_handler *qmp, > - int fd, const char *name) > -{ > - flexarray_t *parameters = NULL; > - libxl_key_value_list args = NULL; > - int rc = 0; > - > - parameters = flexarray_make(2, 1); > - if (!parameters) > - return ERROR_NOMEM; > - flexarray_append_pair(parameters, "fdname", (char*)name); > - args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count); > - if (!args) { > - rc = ERROR_NOMEM; > - goto out; > - } > - > - if (qmp_send_fd(gc, qmp, &args, NULL, NULL, NULL, fd) < 0) { > - rc = ERROR_FAIL; > - } > -out: > - flexarray_free(parameters); > - return rc; > -} > - > -int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd) > +int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename) > { > -#define MIGRATE_FD_NAME "dm-migrate" > libxl__qmp_handler *qmp = NULL; > flexarray_t *parameters = NULL; > libxl_key_value_list args = NULL; > @@ -852,23 +780,19 @@ int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd) > if (!qmp) > return ERROR_FAIL; > > - rc = qmp_getfd(gc, qmp, fd, MIGRATE_FD_NAME); > - if (rc) > - goto out; > - > parameters = flexarray_make(2, 1); > if (!parameters) { > rc = ERROR_NOMEM; > goto out; > } > - flexarray_append_pair(parameters, "uri", "fd:" MIGRATE_FD_NAME); > + flexarray_append_pair(parameters, "filename", (char *)filename); > args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count); > if (!args) { > rc = ERROR_NOMEM; > goto out2; > } > > - rc = qmp_synchronous_send(qmp, "migrate", &args, > + rc = qmp_synchronous_send(qmp, "save_devices", &args, > NULL, NULL, qmp->timeout); > > out2: