[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH V1] on special migration(domain defined, not started yet)



On 08/07/2012 02:00 AM, liguang wrote:
> Hi, Eric
> what do you think of this kludged way of
> off-line migration?
> 
> 在 2012-08-02四的 17:02 +0800,liguang写道:
>>
>> From: liguang <lig fnst cn fujitsu com>
>>
>> a roughly way for offline-migrate (domain defined,
>> not started yet), now can do like this:
>> migrate --hard-migrate --xml dom.xml dom qemu+ssh://target/system
>>
>> this patch will push dom.xml and all disk images to target

Having virsh be able to do this makes it possible to port to older
libvirtd, but I'd really like to first see support for offline migration
directly in the virDomainMigrate* APIs.

>>
>> Signed-off-by: liguang <lig fnst cn fujitsu com>
>> ---
>>  tools/virsh.c |   76
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 76 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index 53d1825..5793233 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -7344,9 +7344,75 @@ static const vshCmdOptDef opts_migrate[] = {
>>      {"dname", VSH_OT_DATA, 0, N_("rename to new name during migration
>> (if supported)")},
>>      {"timeout", VSH_OT_INT, 0, N_("force guest to suspend if live
>> migration exceeds timeout (in seconds)")},
>>      {"xml", VSH_OT_STRING, 0, N_("filename containing updated XML for
>> the target")},
>> +    {"hard-migrate", VSH_OT_BOOL, 0, N_("migration when there's no
>> domain")},

I'd rather see a name like --offline.

>>      {NULL, 0, 0, NULL}
>>  };
>>  
>> +#define VIR_MIGRATE_HARD 1 << 10
>> +#define push_file(file) {                       \
>> +        virAsprintf(&topath, "%s:%s", to, file);            \
>> +        cmd = virCommandNewArgList("scp", file, topath, NULL);  \
>> +        vshPrint(ctl, "pushing %s to %s\n", file, to);          \
>> +        if (virCommandRunAsync(cmd, NULL) < 0 ||                \
>> +            virCommandWait(cmd, NULL) < 0) {                    \

If you're just going to wait for a file, then virCommandRun() is better
than virCommandRunAsync/virCommandWait.

>> +            virshReportError(ctl);                              \
>> +            goto cleanup;                                       \
>> +        }                                                       \
>> +}
>> +
>> +static void
>> +vshMigrateHard(vshControl *ctl, char *doc, char dst[])

const char *, if this function isn't going to alter 'doc' or 'dst'.

>> +{
>> +    xmlDocPtr xml = NULL;
>> +    xmlXPathObjectPtr obj= NULL;
>> +    xmlXPathContextPtr ctxt = NULL;
>> +    xmlNodePtr *disks = NULL;
>> +    virCommandPtr cmd;
>> +    int i = 0, ret = 0;
>> +    int outfd = STDOUT_FILENO;
>> +    int errfd = STDERR_FILENO;
>> +    char *src[] = {NULL}, *to, *topath;
>> +
>> +    if (!vshConnectionUsability(ctl, ctl->conn))
>> +        return;
>> +
>> +    xml = virXMLParseFileCtxt(doc, &ctxt);
>> +    if (!xml) {
>> +        vshError(NULL, "%s", _("Fail to get domain information from"));
>> +        goto cleanup;
>> +    }
>> +
>> +    ret = virXPathNodeSet("./devices/disk", ctxt, &disks);
>> +    if (ret < 0) {
>> +        vshError(NULL, "%s", _("Fail to get disk node"));
>> +        goto cleanup;
>> +    }
>> +
>> +    to = strtok(dst, "/");
>> +    to = strtok(NULL, "/");

strtok() is not thread-safe, and may not be used in libvirt sources.
Run 'make syntax-check' to flag things like this.

>> +    virCommandSetInputFD(cmd, STDIN_FILENO);
>> +    virCommandSetOutputFD(cmd, &outfd);
>> +    virCommandSetErrorFD(cmd, &errfd);
>> +
>> +    push_file(doc);
>> +
>> +    for (i = 0 ; i < ret ; i++) {
>> +        ctxt->node = disks[i];
>> +        src[i] = virXPathString("string(./source/@file"
>> +                                "|./source/@dir"
>> +                                "|./source/@name)", ctxt);
>> +        push_file(src[i]);

Using scp to copy disk images does not seem like the right approach - it
doesn't scale to non-ssh connections.  Rather, we should be using
libvirt API, like virStorageVolUpload().

>> +    }
>> +
>> +cleanup:
>> +    xmlXPathFreeObject(obj);
>> +    xmlXPathFreeContext(ctxt);
>> +    xmlFreeDoc(xml);
>> +    virCommandFree(cmd);
>> +    if (src)
>> +        VIR_FREE(src);    return;

Formatting is off.

>> +}
>> +
>>  static void
>>  doMigrate (void *opaque)
>>  {
>> @@ -7413,12 +7479,22 @@ doMigrate (void *opaque)
>>      if (vshCommandOptBool(cmd, "unsafe"))
>>          flags |= VIR_MIGRATE_UNSAFE;
>>  
>> +    if (vshCommandOptBool(cmd, "hard-migrate")) {
>> +        flags |= VIR_MIGRATE_HARD;

If you aren't calling the libvirt API, then don't stick a random bit in
the 'flags' parameter destined for that API.  Instead, use a new bool
variable to track whether the user is requesting offline migration.

>> +        if (xmlfile == NULL)
>> +            vshError(ctl, _("please specify xmlfile for
>> hard-migrate"));
>> +    }
>>      if (xmlfile &&
>>          virFileReadAll(xmlfile, 8192, &xml) < 0) {
>>          vshError(ctl, _("file '%s' doesn't exist"), xmlfile);
>>          goto out;
>>      }
>>  
>> +    if (flags & VIR_MIGRATE_HARD) {
>> +        vshMigrateHard(ctl, (char *)xmlfile, (char *)desturi);
>> +        goto out;
>> +    }
>> +
>>      if ((flags & VIR_MIGRATE_PEER2PEER) ||
>>          vshCommandOptBool(cmd, "direct")) {
>>          /* For peer2peer migration or direct migration we only expect
>> one URI
>> -- 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]