From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=37302 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ps1Qo-00048w-0k for qemu-devel@nongnu.org; Tue, 22 Feb 2011 18:14:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ps1Qm-0006NI-K4 for qemu-devel@nongnu.org; Tue, 22 Feb 2011 18:14:09 -0500 Received: from mail-vw0-f45.google.com ([209.85.212.45]:43181) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ps1Qm-0006N7-HU for qemu-devel@nongnu.org; Tue, 22 Feb 2011 18:14:08 -0500 Received: by vws19 with SMTP id 19so3315803vws.4 for ; Tue, 22 Feb 2011 15:14:08 -0800 (PST) Message-ID: <4D644343.4050800@codemonkey.ws> Date: Tue, 22 Feb 2011 17:14:11 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <20110222170004.808373778@redhat.com> <20110222170115.710717278@redhat.com> <4D642181.4080509@codemonkey.ws> <20110222210735.GA9372@amt.cnet> <4D64266A.3060106@codemonkey.ws> <20110222230935.GA11082@amt.cnet> In-Reply-To: <20110222230935.GA11082@amt.cnet> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [patch 2/3] Add support for live block copy List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcelo Tosatti Cc: Jes.Sorensen@redhat.com, qemu-devel@nongnu.org, avi@redhat.com On 02/22/2011 05:09 PM, Marcelo Tosatti wrote: > On Tue, Feb 22, 2011 at 03:11:06PM -0600, Anthony Liguori wrote: > >> On 02/22/2011 03:07 PM, Marcelo Tosatti wrote: >> >>> On Tue, Feb 22, 2011 at 02:50:09PM -0600, Anthony Liguori wrote: >>> >>>>> +static int write_commit_file(BdrvCopyState *s) >>>>> +{ >>>>> + char commit_msg[1400]; >>>>> + const char *buf = commit_msg; >>>>> + int len, ret; >>>>> + >>>>> + sprintf(commit_msg, "commit QEMU block_copy %s -> %s\n", s->src_device_name, >>>>> + s->dst_filename); >>>>> + >>>>> + len = strlen(commit_msg); >>>>> + while (len> 0) { >>>>> + ret = write(s->commit_fd, buf, len); >>>>> + if (ret == -1&& errno == EINTR) { >>>>> + continue; >>>>> + } >>>>> + if (ret<= 0) { >>>>> + return -errno; >>>>> + } >>>>> + buf += ret; >>>>> + len -= ret; >>>>> + } >>>>> + >>>>> + if (fsync(s->commit_fd) == -1) { >>>>> + return -errno; >>>>> + } >>>>> >>>>> >>>> This is more or less black magic. What is this commit file used for >>>> and why aren't we using something like a QMP event? >>>> >>> The commit file is considered reliable storage for the result of image >>> switch operation. Think of the following scenario: >>> >>> - mgmt app requests live copy of guests ide1-hd0 >>> >> >from /a/image.img to /b/image.img. >> >>> - mgmt app dies. >>> - guest switches to /b/image.img, /a/image.img is outdated. >>> - guest dies. >>> >>> Notifying the switch via QMP would not be reliable in this case. >>> >> But this is true of any number of operations in QEMU such as hotplug >> where if a management tool dies after requesting hotplug and then >> the guest dies before the management tool reconnects, it's never >> known whether it's truly connected or not. >> > This is a different case. The mgmt tool should be able to safely store > the desired device tree before hotplugging a device. > > And even if you consider that should be done by QEMU, in a config file, > restarting the guest with or without the device is not going to result > in data corruption/loss. > If a guest started writing to a disk and is storing important data there, and then the guest is restarted and the disk (and data) isn't there, I'd classify that as data loss. >> The only way to robustly solve this is for QEMU to maintain a >> stateful config file. A one-off for this particular command doesn't >> seem wise to me. >> > I don't disagree the commit message could be in a config file, but since > that does not exist, a separate file is a workable solution. I disagree. This is something we'll have to maintain forever and what's to stop us from having additional one-off mechanisms to deal with this same problem. -drive already ties into the qemuopts infrastructure and we have readconfig and writeconfig. I don't think we're missing any major pieces to do this in a more proper fashion. Regards, Anthony Liguori > Also, the > separate commit file is not incompatible with future improvements. > >