From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2F6v-0003VK-0s for qemu-devel@nongnu.org; Thu, 03 Nov 2016 06:19:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2F6p-0002SY-Oa for qemu-devel@nongnu.org; Thu, 03 Nov 2016 06:19:05 -0400 From: Juan Quintela In-Reply-To: (Jianjun Duan's message of "Wed, 2 Nov 2016 09:54:51 -0700") References: <1477943636-21024-1-git-send-email-duanj@linux.vnet.ibm.com> <1477943636-21024-2-git-send-email-duanj@linux.vnet.ibm.com> <87oa1y88qu.fsf@emacs.mitica> Reply-To: quintela@redhat.com Date: Thu, 03 Nov 2016 11:18:52 +0100 Message-ID: <87oa1w6f3n.fsf@emacs.mitica> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [QEMU PATCH v10 1/3] migration: extend VMStateInfo List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jianjun Duan Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, dmitry@daynix.com, peter.maydell@linaro.org, kraxel@redhat.com, mst@redhat.com, david@gibson.dropbear.id.au, pbonzini@redhat.com, veroniabahaa@gmail.com, amit.shah@redhat.com, mreitz@redhat.com, kwolf@redhat.com, rth@twiddle.net, aurelien@aurel32.net, leon.alrae@imgtec.com, blauwirbel@gmail.com, mark.cave-ayland@ilande.co.uk, mdroth@linux.vnet.ibm.com, dgilbert@redhat.com Jianjun Duan wrote: > On 11/02/2016 03:40 AM, Juan Quintela wrote: >> Jianjun Duan wrote: >>> Current migration code cannot handle some data structures such as >>> QTAILQ in qemu/queue.h. Here we extend the signatures of put/get >>> in VMStateInfo so that customized handling is supported. >>> >>> Signed-off-by: Jianjun Duan >> >>> +/* VMStateInfo allows customized migration of objects that don't fit in >>> + * any category in VMStateFlags. Additional information can be passed >>> + * into get and put in terms of field and vmdesc parameters. >>> + * For primitive data types such as integer, field and vmdesc parameters >>> + * should be ignored inside get/put. */ >>> struct VMStateInfo { >>> const char *name; >>> - int (*get)(QEMUFile *f, void *pv, size_t size); >>> - void (*put)(QEMUFile *f, void *pv, size_t size); >>> + int (*get)(QEMUFile *f, void *pv, size_t size, VMStateField *field); >>> + void (*put)(QEMUFile *f, void *pv, size_t size, VMStateField *field, >>> + QJSON *vmdesc); >> >> If we are changing put type, I would like to add an error value to it. >> > so we change the return type to int? Yeap. Right now it is impossible to return errors. If we change the prototype, better to just add the return of the error, and that everything return 0 for now. Thanks, Juan. >>> static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, >>> void *opaque, QJSON *vmdesc); >>> @@ -83,6 +84,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, >>> >>> trace_vmstate_load_state(vmsd->name, version_id); >>> if (version_id > vmsd->version_id) { >>> + error_report("%s %s", vmsd->name, "too new"); >>> trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL); >>> return -EINVAL; >>> } >>> @@ -93,6 +95,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, >>> trace_vmstate_load_state_end(vmsd->name, "old path", ret); >>> return ret; >>> } >>> + error_report("%s %s", vmsd->name, "too old"); >>> trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL); >>> return -EINVAL; >>> } >> >> This two are good, but don't belong to this patch, please sent separately. >> > OK. >>> @@ -122,8 +125,10 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, >>> ret = vmstate_load_state(f, field->vmsd, addr, >>> field->vmsd->version_id); >>> } else { >>> - ret = field->info->get(f, addr, size); >>> - >>> + /* field is always passed in. But it should be ignored by >>> + * get when not needed. It is only needed in cases* of >>> + * customized handling, such as migrating QTAILQ. */ >>> + ret = field->info->get(f, addr, size, field); >> >> I would not put this comment here, better on the header when the field >> is declared? Same for the next one. >> > OK. > > Thanks, > Jianjun >>> } >>> if (ret >= 0) { >>> ret = qemu_file_get_error(f); >>> @@ -328,7 +333,11 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, >>> if (field->flags & VMS_STRUCT) { >>> vmstate_save_state(f, field->vmsd, addr, vmdesc_loop); >>> } else { >>> - field->info->put(f, addr, size); >>> + /* field and vmdesc_loop are always passed in. But they >>> + * should be ignored by put when not needed. They are >>> + * only needed in cases f customized handling, such as >>> + * migrating QTAILQ. */ >>> + field->info->put(f, addr, size, field, vmdesc_loop); >>> } >> >> >> Rest of patch is ok. >>