Hi,

Thanks for the quick review!

On Tue, May 1, 2012 at 4:25 AM, Daniel P. Berrange <berra...@redhat.com> wrote:
> On Mon, Apr 30, 2012 at 02:55:06PM -0400, William Jon McCann wrote:
...
>> +    if (!virFileIsDir(old_base) || virFileExists(config_dir)) {
>> +        goto error;
>> +    }
>> +
>> +    /* test if we already attempted to migrate first */
>> +    if (virAsprintf(&updated, "%s/DEPRECATED-DIRECTORY", old_base) < 0) {
>> +        goto error;
>> +    }
>> +    if (virFileExists(updated)) {
>> +        goto error;
>> +    }
>
> I'm not sure that this is actually needed - shouldn't the test
> for existance of 'config_dir' catch this.
>
> In any case, instead of using such a file, I think we should just
> add a symlink from $HOME/.libvirt to $HOME/.config/libvirt. So
> you could replace this with a check to see if old_base is a symlink

That file is only written when we fail to migrate so it wouldn't
really be equivalent to a symlink. I'm not sure adding a symlink is a
good idea really though. We haven't done that for any other type of
migration and it would be a bit misleading here because the config
directory isn't really an exact match for ~/llibvirt and it may
actually cause a problem when an older version is used after
migration.

Perhaps that file isn't necessary if we exit on migration failures though.

Sending an updated patch.

Thanks,
Jon

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to