Re: [Qemu-devel] [PATCH v4 2/8] block: add basic backup support to block driver

[Stefan Hajnoczi]

On Wed, May 22, 2013 at 11:56:45AM +0200, Kevin Wolf wrote:
> Am 22.05.2013 um 11:54 hat Paolo Bonzini geschrieben:
> > Il 22/05/2013 11:38, Kevin Wolf ha scritto:
> > >> +
> > >> +        DPRINTF("brdv_co_backup_cow done C%" PRId64 "\n", start);
> > >> +    }
> > >> +
> > >> +out:
> > >> +    if (bounce_buffer) {
> > >> +        qemu_vfree(bounce_buffer);
> > >> +    }
> > >> +
> > >> +    cow_request_end(&cow_request);
> > >> +
> > >> +    qemu_co_rwlock_unlock(&job->flush_rwlock);
> > >> +
> > >> +    return ret;
> > >> +}
> > >> +
> > >> +static void coroutine_fn backup_before_write_notify(Notifier *notifier,
> > >> +                                                    void *opaque)
> > >> +{
> > >> +    BdrvTrackedRequest *req = opaque;
> > >> +    backup_do_cow(req->bs, req->sector_num, req->nb_sectors);
> > >> +}
> > > 
> > > I don't think you can ignore errors here. Not sure if we can stop the VM
> > > and resume later or something like that, but if we can't, the backup
> > > will be invalid and we must fail the job.
> > 
> > Yes, there is rerror/werror machinery for jobs that this patch is not using.
> 
> This is not enough here. The guest write can't continue before the old
> content is saved to the backup image.

Are you saying just the vm_stop(RUN_STATE_IO_ERROR) call is missing?

I think the reason it's not there is because there's an assumption that
block jobs are in the background and do not affect the guest.  The guest
continues running while the block job is in an error state.

But perhaps we can make the vm_stop() call optional so that drive-backup
can use it.

Stefan