From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45916) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uuil9-0003di-9n for qemu-devel@nongnu.org; Thu, 04 Jul 2013 08:35:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uuil8-0004eH-2U for qemu-devel@nongnu.org; Thu, 04 Jul 2013 08:35:39 -0400 Received: from mail-ea0-x231.google.com ([2a00:1450:4013:c01::231]:63713) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uuil7-0004dI-QE for qemu-devel@nongnu.org; Thu, 04 Jul 2013 08:35:37 -0400 Received: by mail-ea0-f177.google.com with SMTP id j14so749729eak.8 for ; Thu, 04 Jul 2013 05:35:37 -0700 (PDT) Date: Thu, 4 Jul 2013 14:35:33 +0200 From: Stefan Hajnoczi Message-ID: <20130704123533.GA4213@stefanha-thinkpad.redhat.com> References: <1372300908-7546-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1372300908-7546-5-git-send-email-xiawenc@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1372300908-7546-5-git-send-email-xiawenc@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH V3 4/9] qmp: add internal snapshot support in qmp_transaction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: kwolf@redhat.com, phrdina@redhat.com, famz@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, dietmar@proxmox.com On Thu, Jun 27, 2013 at 10:41:43AM +0800, Wenchao Xia wrote: > + /* check whether a snapshot with name exist, no need to check id, since > + name will be checked later to make sure it does not mess up with id. */ > + ret = bdrv_snapshot_find_by_id_and_name(bs, NULL, name, &sn, errp); > + if (error_is_set(errp)) { > + return; > + } > + if (ret) { > + error_setg(errp, > + "Snapshot with name '%s' already exist on device '%s'", s/exist/exists/ > + name, device); > + return; > + } > + > + /* Forbid having a name similar to id, empty name is also forbidden. */ > + if (!snapshot_name_wellformed(name)) { > + error_setg(errp, "Name '%s' on device '%s' is not a valid one", > + name, device); > + return; > + } > + > + /* 3. take the snapshot */ > + sn1 = &state->sn; > + pstrcpy(sn1->name, sizeof(sn1->name), name); > + qemu_gettimeofday(&tv); > + sn1->date_sec = tv.tv_sec; > + sn1->date_nsec = tv.tv_usec * 1000; > + sn1->vm_clock_nsec = qemu_get_clock_ns(vm_clock); > + > + if (bdrv_snapshot_create(bs, sn1) < 0) { > + error_setg(errp, "Failed to create snapshot '%s' on device '%s'", > + name, device); Please use error_setg_errno() to include the bdrv_snapshot_create() error message. > @@ -1009,6 +1010,18 @@ the new image file has the same contents as the current one; QEMU cannot > perform any meaningful check. Typically this is achieved by using the > current image file as the backing file for the new image. > > +On failure, the original disks pre-snapshot attempt will be used. > + > +For internal snapshots, the dictionary contains the device and the snapshot's > +name. If name is a numeric string which will mess up with ID, the request will This is about namespace collision. "Collide" or "conflict" are usually used to describe identical naming problems. Instead of "mess up" I would say something like: The name must not be a numeric string since this collides with snapshot IDs and an error will be returned. > +be rejected. For example, name "99" is not a valid name. If an internal > +snapshot matching name already exists, the request will be also rejected. Only > +some image formats support it, for example, qcow2, rbd, and sheepdog. > + > +On failure, qemu will try delete new created internal snapshot in the s/new created/the newly created/ > +transaction. When I/O error causes deletion failure, the user needs to fix it When an I/O error occurs during deletion, ...