From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 16/27] tools/libxl: Infrastructure for reading a libxl migration v2 stream Date: Tue, 16 Jun 2015 16:46:17 +0100 Message-ID: <558044C9.3000904@citrix.com> References: <1434375880-30914-1-git-send-email-andrew.cooper3@citrix.com> <1434375880-30914-17-git-send-email-andrew.cooper3@citrix.com> <1434465119.13744.196.camel@citrix.com> <55803A64.5040601@citrix.com> <1434468928.13744.234.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1434468928.13744.234.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Ross Lagerwall , Wei Liu , Yang Hongyang , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 16/06/15 16:35, Ian Campbell wrote: > >>>> + if (dc->writefd == -1) { >>>> + ret = ERROR_FAIL; >>>> + LOGE(ERROR, "Unable to open '%s'", path); >>>> + goto err; >>>> + } >>>> + dc->maxsz = dc->bytes_to_read = rec_hdr->length - sizeof(*emu_hdr); >>>> + stream->expected_len = dc->used = 0; >>> expecting 0? This differs from the pattern common everywhere else and >>> I'm not sure why. >> The datacopier has been overloaded so many times, it is messy to use. >> >> In this case, we are splicing from read fd to a write fd, rather than to >> a local buffer. >> >> Therefore, when the IO is complete, we expect 0 bytes in the local >> buffer, as all data should end up in the fd. > I think using 2 or more data copiers to cover the different > configurations might help? You can still reuse one for the normal record > processing but a separate dedicated one for writing the emu to a file > might iron out a wrinkle. I specifically do not want to risk setting two dc's running at the same time with the same readfd. As all of this code is reading from a single readfd, I have specifically avoided having multiple dc structures lying around. > >>> And given that why not handle this in some central place rather than in >>> the emulator only place? >> Experimentally, some versions of Qemu barf if they have trailing zeros >> in save file. I think they expect to find eof() on a qemu record boundary. > What I was suggesting was to do the padding in the core, where it would > often be a zero nop, but would save mistakes (or duplication) if some > other record also needs such handling in the future. I don't see an easy way of doing that, given the current divergence in setting the dcs up in the first place. ~Andrew