From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46707) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brM7K-0008Eg-RO for qemu-devel@nongnu.org; Tue, 04 Oct 2016 05:34:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brM7I-0005g2-El for qemu-devel@nongnu.org; Tue, 04 Oct 2016 05:34:29 -0400 Date: Tue, 4 Oct 2016 11:34:18 +0200 From: Kevin Wolf Message-ID: <20161004093418.GC5316@noname.str.redhat.com> References: <57EE9CA4.6010801@virtuozzo.com> <57EEB604.1090908@virtuozzo.com> <20161003131151.GB31993@stefanha-x1.localdomain> <20161004092349.GD4587@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="M9NhX3UHpAaciwkO" Content-Disposition: inline In-Reply-To: <20161004092349.GD4587@stefanha-x1.localdomain> Subject: Re: [Qemu-devel] backup notifier fail policy List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: John Snow , Vladimir Sementsov-Ogievskiy , Fam Zheng , qemu block , Jeff Cody , qemu-devel , "Denis V. Lunev" --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 04.10.2016 um 11:23 hat Stefan Hajnoczi geschrieben: > On Mon, Oct 03, 2016 at 02:07:34PM -0400, John Snow wrote: > >=20 > >=20 > > On 10/03/2016 09:11 AM, Stefan Hajnoczi wrote: > > > On Fri, Sep 30, 2016 at 09:59:16PM +0300, Vladimir Sementsov-Ogievski= y wrote: > > > > On 30.09.2016 20:11, Vladimir Sementsov-Ogievskiy wrote: > > > > > Hi all! > > > > >=20 > > > > > Please, can somebody explain me, why we fail guest request in cas= e of io > > > > > error in write notifier? I think guest consistency is more import= ant > > > > > than success of unfinished backup. Or, what am I missing? > > > > >=20 > > > > > I'm saying about this code: > > > > >=20 > > > > > static int coroutine_fn backup_before_write_notify( > > > > > NotifierWithReturn *notifier, > > > > > void *opaque) > > > > > { > > > > > BackupBlockJob *job =3D container_of(notifier, BackupBlockJob, > > > > > before_write); > > > > > BdrvTrackedRequest *req =3D opaque; > > > > > int64_t sector_num =3D req->offset >> BDRV_SECTOR_BITS; > > > > > int nb_sectors =3D req->bytes >> BDRV_SECTOR_BITS; > > > > >=20 > > > > > assert(req->bs =3D=3D blk_bs(job->common.blk)); > > > > > assert((req->offset & (BDRV_SECTOR_SIZE - 1)) =3D=3D 0); > > > > > assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) =3D=3D 0); > > > > >=20 > > > > > return backup_do_cow(job, sector_num, nb_sectors, NULL, true); > > > > > } > > > > >=20 > > > > > So, what about something like > > > > >=20 > > > > > ret =3D backup_do_cow(job, ... > > > > > if (ret < 0 && job->notif_ret =3D=3D 0) { > > > > > job->notif_ret =3D ret; > > > > > } > > > > >=20 > > > > > return 0; > > > > >=20 > > > > > and fail block job if notif_ret < 0 in other places of backup cod= e? > > > > >=20 > > > >=20 > > > > And second question about notifiers in backup block job. If block j= ob is > > > > paused, notifiers still works and can copy data. Is it ok? So, user= thinks > > > > that job is paused, so he can do something with target disk.. But r= eally, > > > > this 'something' will race with write-notifiers. So, what assumptio= ns may > > > > user actually have about paused backup job? Is there any agreements= ? Also, > > > > on query-block-jobs we will see job.busy =3D false, when actually > > > > copy-on-write may be in flight.. > > >=20 > > > I agree that the job should fail and the guest continues running. > > >=20 > > > The backup job cannot do the usual ENOSPC stop/resume error handling > > > since we lose snapshot consistency once guest writes are allowed to > > > proceed. Backup errors need to be fatal, resuming is usually not > > > possible. The user will have to retry the backup operation. > > >=20 > > > Stefan > > >=20 > >=20 > > If we fail and intercept the error for the backup write and HALT at that > > point, why would we lose consistency? If the backup write failed before= we > > allowed the guest write to proceed, that data should still be there on = disk, > > no? >=20 > I missed that there are two separate error handling approaches used in > block/backup.c: >=20 > 1. In the write notifier I/O errors are treated as if the guest write > failed. >=20 > 2. In the backup_run() loop I/O errors affect the block job's error > status. >=20 > I was thinking of case #2 instead of case #1. >=20 > > Eh, regardless: If we're not using a STOP policy, it seems like the rig= ht > > thing to do is definitely to just fail the backup instead of failing the > > write. >=20 > Even with a -drive werror=3Dstop policy the user probably doesn't want > guest downtime if writing to the backup target fails. That's a policy decision that ultimately only the user can make. For one user, it might be preferable to cancel the backup and keep the VM running, but for another user it may be more important to keep a consistent snapshot of the point in time when the backup job was started than keeping the VM running. Kevin --M9NhX3UHpAaciwkO Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJX83eaAAoJEH8JsnLIjy/WBT8QAMmfNToHCKaz+qfPiDQfVmvw DshrikAexrFQc2+vU2ZYCi0g5m6+Nvk+6t9OJn+49NdHOCp/494ZgeThsboGCpdU 9BXadOhfPOxy3Gt3/avKp7SrVNFP4f8shardTXIQ4Aa2Zg5g+23dmBP7h40mMpUj bQ7tKzbIjuymDXhmje4+1KQVdrV08e96Hc0KvePxe6xegttweoNMb1ug+rXO9S6I yJa7fvHwhQxQeDSQwsa7e42v8q6gztVvEtSqLkp3WOnuHk3zsCWERHsh9UvLJxj9 8neD9YuvcYsi/zfLYRd24u2vpEChqJSaTK6tTqDRdsmZbVbq1bRYKg4Ako+JaZuY io4FnfLqJMCKHS6w+61le523T7KAZTU3BscxytVxbvWOKj1tbFX4r3/G67KRYolN 5IdL2UqNSJRPUZlboeYXttWl7s3P0NBDPxAFsuvcIIvBGTfrk57JgyZ8RU1lh7FG izDiGKvVEgNj+6B3zBGbjQam5ShR/CBmK/ffeE1HdJOmvF7mLGoj8y2ac2x11zTF wbjw4JtCioJaBxJB9bl5ucfyxRMimucp5swoCnuYcgPRYAdms29nc1ft35C+Yflm WNCbIeEzGwX9W3lxlHhEauBySyZp3be4+dsieNYdJzx2YK4sww0w9cdeOwgrxzcw JvZRosVNiNGqhWUI4/yM =pqTB -----END PGP SIGNATURE----- --M9NhX3UHpAaciwkO--