Daniel P. Berrange wrote:
> On Wed, May 20, 2009 at 12:06:34PM +0200, Daniel Veillard wrote:
>> On Wed, May 20, 2009 at 11:15:14AM +0200, Chris Lalancette wrote:
>>> As pointed out by ??ukasz Mierzwa <l.mierzwa@xxxxxxxxx>, it would be nice if
>>> there was an option to automatically make a domain persistent on the destination
>>> during a live migration. The attached patch adds this simple capability. Note
>>> that this has to be applied on top of my previous secure migration patch,
>>> otherwise you'll have conflicts.
>>>
>>> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
>> Patch looks fine to me, but
>>
>>> @@ -5291,6 +5292,32 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
>>> * object, but if no, clean up the empty qemu process.
>>> */
>>> if (retcode == 0) {
>>> + if (flags & VIR_MIGRATE_PERSISTENT) {
>>> + if (vm->persistent)
>>> + newVM = 0;
>>> + vm->persistent = 1;
>>> +
>>> + if (virDomainSaveConfig(dconn, driver->configDir, vm->def) < 0) {
>>> + /* Hmpf. Migration was successful, but making it persistent
>>> + * was not. If we report successful, then when this domain
>>> + * shuts down, management tools are in for a surprise. On the
>>> + * other hand, if we report failure, then the management tools
>>> + * might try to restart the domain on the source side, even
>>> + * though the domain is actually running on the destination.
>>> + * Return a NULL dom pointer, and hope that this is a rare
>>> + * situation and management tools are smart.
>>> + */
>>> + vm = NULL;
>>> + goto cleanup;
>>> + }
>> Yup that's a hard decision here, I wonder if some asynchronous event
>> shoul not be generated, at least make sure an error is propagated in
>> some ways.
>
> I think it is preferable to always return the VM object in this scenario,
> because it is more important for the mgmt tool to know that the migration
> itself has completed successfully and the VM is now running on the dest.
OK, yeah, that's fine. I just wasn't sure. I'll respin that patch. I guess
I'll also dump something to the logs, so at least a post-mortem analysis will
have some shot at seeing what happened.
>
> I think there is an argument for adding an explicit API
>
> virDomainIsPersistent(virDomainPtr)
>
> to allow an app to determine whether it has a config or not. Even if
> the mgmt tool did not set the 'VIR_MIGRATE_PERSISTENT' flag, they
> may still need to find out whether it is persistent or not.
Yeah, true, although maybe we want a call that is a bit more general. Something
like "virDomainProperties()", one of which is whether it is persistent or not.
--
Chris Lalancette
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list