Hi, Thanks for the quick review! On Tue, May 1, 2012 at 4:25 AM, Daniel P. Berrange <berrange@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