What does libvirt actually do in the monitor prior to migration completing on the destination? The least invasive way of doing delayed open of block devices is probably to make -incoming create a monitor and run a main loop before the block devices (and full device model) is initialized. Since this isolates the changes strictly to migration, I'd feel okay doing this for 1.0 (although it might need to be in the stable branch).

On 11/10/2011 12:42 PM, Daniel P. Berrange wrote: >On Thu, Nov 10, 2011 at 12:27:30PM -0600, Anthony Liguori wrote: >>What does libvirt actually do in the monitor prior to migration >>completing on the destination? The least invasive way of doing >>delayed open of block devices is probably to make -incoming create a >>monitor and run a main loop before the block devices (and full >>device model) is initialized. Since this isolates the changes >>strictly to migration, I'd feel okay doing this for 1.0 (although it >>might need to be in the stable branch). > >The way migration works with libvirt wrt QEMU interactions is now >as follows > > 1. Destination. > Run qemu -incoming ...args... > Query chardevs via monitor > Query vCPU threads via monitor > Set disk / vnc passwords Since RHEL carries Juan's patch, and Juan's patch doesn't handle disk passwords gracefully, how does libvirt cope with that?

On 11/10/2011 02:55 AM, Avi Kivity wrote: > If we have to delay the release for a month to get it right, we should. > Not that I think we have to. > Adding libvirt to the discussion. What does libvirt actually do in the monitor prior to migration completing on the destination? The least invasive way of doing delayed open of block devices is probably to make -incoming create a monitor and run a main loop before the block devices (and full device model) is initialized. Since this isolates the changes strictly to migration, I'd feel okay doing this for 1.0 (although it might need to be in the stable branch).

I know a monitor can run like this as I've done it before but some of the commands will not behave as expected so it's pretty important to be comfortable with what commands are actually being used in this mode. Regards, Anthony Liguori

Am 10.11.2011 22:30, schrieb Anthony Liguori: > Live migration with qcow2 or any other image format is just not going to work > right now even with proper clustered storage. I think doing a block level flush > cache interface and letting block devices decide how to do it is the best approach. I would really prefer reusing the existing open/close code. It means less (duplicated) code, is existing code that is well tested and doesn't make migration much of a special case.

If you want to avoid reopening the file on the OS level, we can reopen only the topmost layer (i.e. the format, but not the protocol) for now and in 1.1 we can use bdrv_reopen().

Kevin

Am 11.11.2011 15:03, schrieb Anthony Liguori: > On 11/11/2011 04:15 AM, Kevin Wolf wrote: >> Am 10.11.2011 22:30, schrieb Anthony Liguori: >>> Live migration with qcow2 or any other image format is just not going to work >>> right now even with proper clustered storage. I think doing a block level flush >>> cache interface and letting block devices decide how to do it is the best approach. >> >> I would really prefer reusing the existing open/close code. It means >> less (duplicated) code, is existing code that is well tested and doesn't >> make migration much of a special case. > > Just to be clear, reopen only addresses image format migration. It does not > address NFS migration since it doesn't guarantee close-to-open semantics. Yes. But image formats are the only thing that is really completely broken today. For NFS etc. we can tell users to use cache=none/directsync and they will be good. There is no such option that makes image formats safe. > The problem I have with the reopen patches are that they introduce regressions > and change at semantics for a management tool. If you look at the libvirt > workflow with encrypted disks, it would break with the reopen patches. Yes, this is nasty. But on the other hand: Today migration is broken for all qcow2 images, with the reopen it's only broken for encrypted ones. Certainly an improvement, even though there's still a bug left.

>> If you want to avoid reopening the file on the OS level, we can reopen >> only the topmost layer (i.e. the format, but not the protocol) for now >> and in 1.1 we can use bdrv_reopen(). > > I don't view not supporting migration with image formats as a regression as it's > never been a feature we've supported. While there might be confusion about > support around NFS, I think it's always been clear that image formats cannot be > used. > > Given that, I don't think this is a candidate for 1.0. Nobody says it's a regression, but it's a bad bug and you're blocking a solution for it for over a year now because the solution isn't perfect enough in your eyes. :-(

Kevin

Am 11.11.2011 15:35, schrieb Anthony Liguori: > This is not a bug fix, this is a new feature. We're long past feature freeze. > It's not a simple and obvious fix either. It only partially fixes the problem > and introduces other problems. It's not a good candidate for making an > exception at this stage in the release. > > [1] http://mid.gmane.org/cover.1294150511.git.quintela@redhat.com Then please send a fix that fails migration with non-raw images. Not breaking images silently during migration is critical for 1.0, IMO.

Kevin

On 11/11/2011 04:03 PM, Anthony Liguori wrote: > > I don't view not supporting migration with image formats as a > regression as it's never been a feature we've supported. While there > might be confusion about support around NFS, I think it's always been > clear that image formats cannot be used. Was there ever a statement to that effect? It was never clear to me and I doubt it was clear to anyone.

> > Given that, I don't think this is a candidate for 1.0. > Let's just skip 1.0 and do 1.1 instead.

On 11/12/2011 03:39 PM, Anthony Liguori wrote: > On 11/12/2011 04:27 AM, Avi Kivity wrote: >> On 11/11/2011 04:03 PM, Anthony Liguori wrote: >>> >>> I don't view not supporting migration with image formats as a >>> regression as it's never been a feature we've supported. While there >>> might be confusion about support around NFS, I think it's always been >>> clear that image formats cannot be used. >> >> Was there ever a statement to that effect? It was never clear to me and >> I doubt it was clear to anyone. > > You literally reviewed a patch who's subject was "block: allow > migration to work with image files"[1] that explained in gory detail > what the problem was. > > [1] http://mid.gmane.org/4C8CAD7C.5020102@redhat.com > Isn't a patch fixing a problem with migrating image files a statement that we do support migrating image files?

On 11/11/2011 12:15 PM, Kevin Wolf wrote: > Am 10.11.2011 22:30, schrieb Anthony Liguori: >> Live migration with qcow2 or any other image format is just not going to work >> right now even with proper clustered storage. I think doing a block level flush >> cache interface and letting block devices decide how to do it is the best approach. > > I would really prefer reusing the existing open/close code. It means > less (duplicated) code, is existing code that is well tested and doesn't > make migration much of a special case. > > If you want to avoid reopening the file on the OS level, we can reopen > only the topmost layer (i.e. the format, but not the protocol) for now > and in 1.1 we can use bdrv_reopen(). Intuitively I dislike _reopen style interfaces. If the second open yields different results from the first, does it invalidate any computations in between?

What's wrong with just delaying the open?

Am 12.11.2011 11:25, schrieb Avi Kivity: > On 11/11/2011 12:15 PM, Kevin Wolf wrote: >> Am 10.11.2011 22:30, schrieb Anthony Liguori: >>> Live migration with qcow2 or any other image format is just not going to work >>> right now even with proper clustered storage. I think doing a block level flush >>> cache interface and letting block devices decide how to do it is the best approach. >> >> I would really prefer reusing the existing open/close code. It means >> less (duplicated) code, is existing code that is well tested and doesn't >> make migration much of a special case. >> >> If you want to avoid reopening the file on the OS level, we can reopen >> only the topmost layer (i.e. the format, but not the protocol) for now >> and in 1.1 we can use bdrv_reopen(). > > Intuitively I dislike _reopen style interfaces. If the second open > yields different results from the first, does it invalidate any > computations in between? Not sure what results and what computation you mean,

but let me clarify a bit about bdrv_reopen: The main purpose of bdrv_reopen() is to change flags, for example toggle O_SYNC during runtime in order to allow the guest to toggle WCE. This doesn't necessarily mean a close()/open() sequence if there are other means to change the flags, like fcntl() (or even using other protocols than files). The idea here was to extend this to invalidate all caches if some specific flag is set. As you don't change any other flag, this will usually not be a reopen on a lower level. If we need to use open() though, and it fails (this is really the only "different" result that comes to mind)

then bdrv_reopen() would fail and the old fd would stay in use. Migration would have to fail, but I don't think this case is ever needed for reopening after migration.

> What's wrong with just delaying the open? Nothing, except that with today's code it's harder to do.

On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: > On 11/11/2011 12:15 PM, Kevin Wolf wrote: > > Am 10.11.2011 22:30, schrieb Anthony Liguori: > > > Live migration with qcow2 or any other image format is just not going to work > > > right now even with proper clustered storage. I think doing a block level flush > > > cache interface and letting block devices decide how to do it is the best approach. > > > > I would really prefer reusing the existing open/close code. It means > > less (duplicated) code, is existing code that is well tested and doesn't > > make migration much of a special case. > > > > If you want to avoid reopening the file on the OS level, we can reopen > > only the topmost layer (i.e. the format, but not the protocol) for now > > and in 1.1 we can use bdrv_reopen(). > > > > Intuitively I dislike _reopen style interfaces. If the second open > yields different results from the first, does it invalidate any > computations in between? > > What's wrong with just delaying the open? If you delay the 'open' until the mgmt app issues 'cont', then you loose the ability to rollback to the source host upon open failure for most deployed versions of libvirt. We only fairly recently switched to a five stage migration handshake to cope with rollback when 'cont' fails. Daniel

-- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: > On Mon, Nov 14, 2011 at 10:16:10AM +0000, Daniel P. Berrange wrote: >> On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: >>> On 11/11/2011 12:15 PM, Kevin Wolf wrote: >>>> Am 10.11.2011 22:30, schrieb Anthony Liguori: >>>>> Live migration with qcow2 or any other image format is just not going to work >>>>> right now even with proper clustered storage. I think doing a block level flush >>>>> cache interface and letting block devices decide how to do it is the best approach. >>>> >>>> I would really prefer reusing the existing open/close code. It means >>>> less (duplicated) code, is existing code that is well tested and doesn't >>>> make migration much of a special case. >>>> >>>> If you want to avoid reopening the file on the OS level, we can reopen >>>> only the topmost layer (i.e. the format, but not the protocol) for now >>>> and in 1.1 we can use bdrv_reopen(). >>>> >>> >>> Intuitively I dislike _reopen style interfaces. If the second open >>> yields different results from the first, does it invalidate any >>> computations in between? >>> >>> What's wrong with just delaying the open? >> >> If you delay the 'open' until the mgmt app issues 'cont', then you loose >> the ability to rollback to the source host upon open failure for most >> deployed versions of libvirt. We only fairly recently switched to a five >> stage migration handshake to cope with rollback when 'cont' fails. >> >> Daniel > > I guess reopen can fail as well, so this seems to me to be an important > fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent.

On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: > Am 14.11.2011 12:08, schrieb Daniel P. Berrange: > > On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: > >> On Mon, Nov 14, 2011 at 10:16:10AM +0000, Daniel P. Berrange wrote: > >>> On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: > >>>> On 11/11/2011 12:15 PM, Kevin Wolf wrote: > >>>>> Am 10.11.2011 22:30, schrieb Anthony Liguori: > >>>>>> Live migration with qcow2 or any other image format is just not going to work > >>>>>> right now even with proper clustered storage. I think doing a block level flush > >>>>>> cache interface and letting block devices decide how to do it is the best approach. > >>>>> > >>>>> I would really prefer reusing the existing open/close code. It means > >>>>> less (duplicated) code, is existing code that is well tested and doesn't > >>>>> make migration much of a special case. > >>>>> > >>>>> If you want to avoid reopening the file on the OS level, we can reopen > >>>>> only the topmost layer (i.e. the format, but not the protocol) for now > >>>>> and in 1.1 we can use bdrv_reopen(). > >>>>> > >>>> > >>>> Intuitively I dislike _reopen style interfaces. If the second open > >>>> yields different results from the first, does it invalidate any > >>>> computations in between? > >>>> > >>>> What's wrong with just delaying the open? > >>> > >>> If you delay the 'open' until the mgmt app issues 'cont', then you loose > >>> the ability to rollback to the source host upon open failure for most > >>> deployed versions of libvirt. We only fairly recently switched to a five > >>> stage migration handshake to cope with rollback when 'cont' fails. > >>> > >>> Daniel > >> > >> I guess reopen can fail as well, so this seems to me to be an important > >> fix but not a blocker. > > > > If if the initial open succeeds, then it is far more likely that a later > > re-open will succeed too, because you have already elminated the possibility > > of configuration mistakes, and will have caught most storage runtime errors > > too. So there is a very significant difference in reliability between doing > > an 'open at startup + reopen at cont' vs just 'open at cont' > > > > Based on the bug reports I see, we want to be very good at detecting and > > gracefully handling open errors because they are pretty frequent. > > Do you have some more details on the kind of errors? Missing files, > permissions, something like this? Or rather something related to the > actual content of an image file? Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI setup. Access permissions due to incorrect user / group setup, or read only mounts, or SELinux denials. Actual I/O errors are less common and are not so likely to cause QEMU to fail to start any, since QEMU is likely to just report them to the guest OS instead. Daniel

-- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: > On Mon, Nov 14, 2011 at 11:29:18AM +0000, Daniel P. Berrange wrote: > > On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: > > > Am 14.11.2011 12:08, schrieb Daniel P. Berrange: > > > > On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: > > > >> On Mon, Nov 14, 2011 at 10:16:10AM +0000, Daniel P. Berrange wrote: > > > >>> On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: > > > >>>> On 11/11/2011 12:15 PM, Kevin Wolf wrote: > > > >>>>> Am 10.11.2011 22:30, schrieb Anthony Liguori: > > > >>>>>> Live migration with qcow2 or any other image format is just not going to work > > > >>>>>> right now even with proper clustered storage. I think doing a block level flush > > > >>>>>> cache interface and letting block devices decide how to do it is the best approach. > > > >>>>> > > > >>>>> I would really prefer reusing the existing open/close code. It means > > > >>>>> less (duplicated) code, is existing code that is well tested and doesn't > > > >>>>> make migration much of a special case. > > > >>>>> > > > >>>>> If you want to avoid reopening the file on the OS level, we can reopen > > > >>>>> only the topmost layer (i.e. the format, but not the protocol) for now > > > >>>>> and in 1.1 we can use bdrv_reopen(). > > > >>>>> > > > >>>> > > > >>>> Intuitively I dislike _reopen style interfaces. If the second open > > > >>>> yields different results from the first, does it invalidate any > > > >>>> computations in between? > > > >>>> > > > >>>> What's wrong with just delaying the open? > > > >>> > > > >>> If you delay the 'open' until the mgmt app issues 'cont', then you loose > > > >>> the ability to rollback to the source host upon open failure for most > > > >>> deployed versions of libvirt. We only fairly recently switched to a five > > > >>> stage migration handshake to cope with rollback when 'cont' fails. > > > >>> > > > >>> Daniel > > > >> > > > >> I guess reopen can fail as well, so this seems to me to be an important > > > >> fix but not a blocker. > > > > > > > > If if the initial open succeeds, then it is far more likely that a later > > > > re-open will succeed too, because you have already elminated the possibility > > > > of configuration mistakes, and will have caught most storage runtime errors > > > > too. So there is a very significant difference in reliability between doing > > > > an 'open at startup + reopen at cont' vs just 'open at cont' > > > > > > > > Based on the bug reports I see, we want to be very good at detecting and > > > > gracefully handling open errors because they are pretty frequent. > > > > > > Do you have some more details on the kind of errors? Missing files, > > > permissions, something like this? Or rather something related to the > > > actual content of an image file? > > > > Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI > > setup. Access permissions due to incorrect user / group setup, or read > > only mounts, or SELinux denials. Actual I/O errors are less common and > > are not so likely to cause QEMU to fail to start any, since QEMU is > > likely to just report them to the guest OS instead. > > Do you run qemu with -S, then give a 'cont' command to start it? Yes Daniel

-- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: > On Mon, Nov 14, 2011 at 11:29:18AM +0000, Daniel P. Berrange wrote: > > On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: > > > Am 14.11.2011 12:08, schrieb Daniel P. Berrange: > > > > On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: > > > >> On Mon, Nov 14, 2011 at 10:16:10AM +0000, Daniel P. Berrange wrote: > > > >>> On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: > > > >>>> On 11/11/2011 12:15 PM, Kevin Wolf wrote: > > > >>>>> Am 10.11.2011 22:30, schrieb Anthony Liguori: > > > >>>>>> Live migration with qcow2 or any other image format is just not going to work > > > >>>>>> right now even with proper clustered storage. I think doing a block level flush > > > >>>>>> cache interface and letting block devices decide how to do it is the best approach. > > > >>>>> > > > >>>>> I would really prefer reusing the existing open/close code. It means > > > >>>>> less (duplicated) code, is existing code that is well tested and doesn't > > > >>>>> make migration much of a special case. > > > >>>>> > > > >>>>> If you want to avoid reopening the file on the OS level, we can reopen > > > >>>>> only the topmost layer (i.e. the format, but not the protocol) for now > > > >>>>> and in 1.1 we can use bdrv_reopen(). > > > >>>>> > > > >>>> > > > >>>> Intuitively I dislike _reopen style interfaces. If the second open > > > >>>> yields different results from the first, does it invalidate any > > > >>>> computations in between? > > > >>>> > > > >>>> What's wrong with just delaying the open? > > > >>> > > > >>> If you delay the 'open' until the mgmt app issues 'cont', then you loose > > > >>> the ability to rollback to the source host upon open failure for most > > > >>> deployed versions of libvirt. We only fairly recently switched to a five > > > >>> stage migration handshake to cope with rollback when 'cont' fails. > > > >>> > > > >>> Daniel > > > >> > > > >> I guess reopen can fail as well, so this seems to me to be an important > > > >> fix but not a blocker. > > > > > > > > If if the initial open succeeds, then it is far more likely that a later > > > > re-open will succeed too, because you have already elminated the possibility > > > > of configuration mistakes, and will have caught most storage runtime errors > > > > too. So there is a very significant difference in reliability between doing > > > > an 'open at startup + reopen at cont' vs just 'open at cont' > > > > > > > > Based on the bug reports I see, we want to be very good at detecting and > > > > gracefully handling open errors because they are pretty frequent. > > > > > > Do you have some more details on the kind of errors? Missing files, > > > permissions, something like this? Or rather something related to the > > > actual content of an image file? > > > > Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI > > setup. Access permissions due to incorrect user / group setup, or read > > only mounts, or SELinux denials. Actual I/O errors are less common and > > are not so likely to cause QEMU to fail to start any, since QEMU is > > likely to just report them to the guest OS instead. > > Do you run qemu with -S, then give a 'cont' command to start it? Yes Daniel

On Mon, Nov 14, 2011 at 01:56:36PM +0200, Michael S. Tsirkin wrote: > On Mon, Nov 14, 2011 at 11:37:27AM +0000, Daniel P. Berrange wrote: > > On Mon, Nov 14, 2011 at 01:34:15PM +0200, Michael S. Tsirkin wrote: > > > On Mon, Nov 14, 2011 at 11:29:18AM +0000, Daniel P. Berrange wrote: > > > > On Mon, Nov 14, 2011 at 12:21:53PM +0100, Kevin Wolf wrote: > > > > > Am 14.11.2011 12:08, schrieb Daniel P. Berrange: > > > > > > On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: > > > > > >> On Mon, Nov 14, 2011 at 10:16:10AM +0000, Daniel P. Berrange wrote: > > > > > >>> On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: > > > > > >>>> On 11/11/2011 12:15 PM, Kevin Wolf wrote: > > > > > >>>>> Am 10.11.2011 22:30, schrieb Anthony Liguori: > > > > > >>>>>> Live migration with qcow2 or any other image format is just not going to work > > > > > >>>>>> right now even with proper clustered storage. I think doing a block level flush > > > > > >>>>>> cache interface and letting block devices decide how to do it is the best approach. > > > > > >>>>> > > > > > >>>>> I would really prefer reusing the existing open/close code. It means > > > > > >>>>> less (duplicated) code, is existing code that is well tested and doesn't > > > > > >>>>> make migration much of a special case. > > > > > >>>>> > > > > > >>>>> If you want to avoid reopening the file on the OS level, we can reopen > > > > > >>>>> only the topmost layer (i.e. the format, but not the protocol) for now > > > > > >>>>> and in 1.1 we can use bdrv_reopen(). > > > > > >>>>> > > > > > >>>> > > > > > >>>> Intuitively I dislike _reopen style interfaces. If the second open > > > > > >>>> yields different results from the first, does it invalidate any > > > > > >>>> computations in between? > > > > > >>>> > > > > > >>>> What's wrong with just delaying the open? > > > > > >>> > > > > > >>> If you delay the 'open' until the mgmt app issues 'cont', then you loose > > > > > >>> the ability to rollback to the source host upon open failure for most > > > > > >>> deployed versions of libvirt. We only fairly recently switched to a five > > > > > >>> stage migration handshake to cope with rollback when 'cont' fails. > > > > > >>> > > > > > >>> Daniel > > > > > >> > > > > > >> I guess reopen can fail as well, so this seems to me to be an important > > > > > >> fix but not a blocker. > > > > > > > > > > > > If if the initial open succeeds, then it is far more likely that a later > > > > > > re-open will succeed too, because you have already elminated the possibility > > > > > > of configuration mistakes, and will have caught most storage runtime errors > > > > > > too. So there is a very significant difference in reliability between doing > > > > > > an 'open at startup + reopen at cont' vs just 'open at cont' > > > > > > > > > > > > Based on the bug reports I see, we want to be very good at detecting and > > > > > > gracefully handling open errors because they are pretty frequent. > > > > > > > > > > Do you have some more details on the kind of errors? Missing files, > > > > > permissions, something like this? Or rather something related to the > > > > > actual content of an image file? > > > > > > > > Missing files due to wrong/missing NFS mounts, or incorrect SAN / iSCSI > > > > setup. Access permissions due to incorrect user / group setup, or read > > > > only mounts, or SELinux denials. Actual I/O errors are less common and > > > > are not so likely to cause QEMU to fail to start any, since QEMU is > > > > likely to just report them to the guest OS instead. > > > > > > Do you run qemu with -S, then give a 'cont' command to start it? > > > > Yes > > OK, so let's go back one step now - how is this related to > 'rollback to source host'? In the old libvirt migration protocol, by the time we run 'cont' on the destination, the source QEMU has already been killed off, so there's nothing to resume on failure. Daniel

-- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Nov 14, 2011 at 12:24:22PM +0200, Michael S. Tsirkin wrote: > On Mon, Nov 14, 2011 at 10:16:10AM +0000, Daniel P. Berrange wrote: > > On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: > > > On 11/11/2011 12:15 PM, Kevin Wolf wrote: > > > > Am 10.11.2011 22:30, schrieb Anthony Liguori: > > > > > Live migration with qcow2 or any other image format is just not going to work > > > > > right now even with proper clustered storage. I think doing a block level flush > > > > > cache interface and letting block devices decide how to do it is the best approach. > > > > > > > > I would really prefer reusing the existing open/close code. It means > > > > less (duplicated) code, is existing code that is well tested and doesn't > > > > make migration much of a special case. > > > > > > > > If you want to avoid reopening the file on the OS level, we can reopen > > > > only the topmost layer (i.e. the format, but not the protocol) for now > > > > and in 1.1 we can use bdrv_reopen(). > > > > > > > > > > Intuitively I dislike _reopen style interfaces. If the second open > > > yields different results from the first, does it invalidate any > > > computations in between? > > > > > > What's wrong with just delaying the open? > > > > If you delay the 'open' until the mgmt app issues 'cont', then you loose > > the ability to rollback to the source host upon open failure for most > > deployed versions of libvirt. We only fairly recently switched to a five > > stage migration handshake to cope with rollback when 'cont' fails. > > > > Daniel > > I guess reopen can fail as well, so this seems to me to be an important > fix but not a blocker. If if the initial open succeeds, then it is far more likely that a later re-open will succeed too, because you have already elminated the possibility of configuration mistakes, and will have caught most storage runtime errors too. So there is a very significant difference in reliability between doing an 'open at startup + reopen at cont' vs just 'open at cont' Based on the bug reports I see, we want to be very good at detecting and gracefully handling open errors because they are pretty frequent. Regards, Daniel

-- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 11/14/2011 04:16 AM, Daniel P. Berrange wrote: > On Sat, Nov 12, 2011 at 12:25:34PM +0200, Avi Kivity wrote: >> On 11/11/2011 12:15 PM, Kevin Wolf wrote: >>> Am 10.11.2011 22:30, schrieb Anthony Liguori: >>>> Live migration with qcow2 or any other image format is just not going to work >>>> right now even with proper clustered storage. I think doing a block level flush >>>> cache interface and letting block devices decide how to do it is the best approach. >>> >>> I would really prefer reusing the existing open/close code. It means >>> less (duplicated) code, is existing code that is well tested and doesn't >>> make migration much of a special case. >>> >>> If you want to avoid reopening the file on the OS level, we can reopen >>> only the topmost layer (i.e. the format, but not the protocol) for now >>> and in 1.1 we can use bdrv_reopen(). >>> >> >> Intuitively I dislike _reopen style interfaces. If the second open >> yields different results from the first, does it invalidate any >> computations in between? >> >> What's wrong with just delaying the open? > > If you delay the 'open' until the mgmt app issues 'cont', then you loose > the ability to rollback to the source host upon open failure for most > deployed versions of libvirt. We only fairly recently switched to a five > stage migration handshake to cope with rollback when 'cont' fails. Delayed open isn't a panacea. With the series I sent, we should be able to migration with a qcow2 file on coherent shared storage. There are two other cases that we care about: migration with nfs cache!=none and direct attached storage with cache!=none Whether the open is deferred matters less with NFS than if the open happens after the close on the source. To fix NFS cache!=none, we would have to do a bdrv_close() before sending the last byte of migration data and make sure that we bdrv_open() after receiving the last byte of migration data. The problem with this IMHO is it creates a large window where noone has the file open and you're critically vulnerable to losing your VM.

I'm much more in favor of a smarter caching policy. If we can fcntl() our way to O_DIRECT on NFS, that would be fairly interesting. I'm not sure if this is supported today but it's something we could look into adding in the kernel. That way we could force NFS to O_DIRECT during migration which would solve this problem robustly.

Deferred open doesn't help with direct attached storage. There simple is no guarantee that there isn't data in the page cache.

Again, I think defaulting DAS to cache=none|directsync is what makes the most sense here.

We can even add a migration blocker for DAS with cache=on. If we can do dynamic toggling of the cache setting, then that's pretty friendly at the end of the day.