[libvirt] Re: [PATCH] Implement finer grained migration control for Xen

[Maximilian Wilhelm]

Anno domini 2009 Chris Lalancette scripsit:

[...]
> > I'm not totaly sure if there are better ways to handle all the error
> > cases. The current solution seemed to be a same one for me.

> Well, I do think that we need to return a proper error in all cases except for
> the one with the long comment.  I've pointed out where I think we need to add
> error messages below.  Otherwise, I think it looks good.

Ok, will do.

> > +    dom = xenUnifiedDomainLookupByName (dconn, dname);
> > +    if (! dom) {

> You are probably going to want to raise some error here, otherwise the user will
> get back "unknown error", which isn't very helpful.

[...]
> > +        return NULL;
> > +    }
> > +
> > +    if (flags & VIR_MIGRATE_PERSIST_DEST) {
> > +        domain_xml = xenDaemonDomainDumpXML (dom, 0, NULL);
> > +	if (! domain_xml) {
> > +            goto failure;
> > +        }

> This seems whitespace damaged (using tabs instead of spaces), and also needs to
> raise a proper error.

[...]
> > +    /* Ignore the persist_dest flag here */
> > +    if (flags & VIR_MIGRATE_PERSIST_DEST) {
> > +        flags &= ~VIR_MIGRATE_PERSIST_DEST;
> > +    }

> Again a nit, but I think the libvirt style currently is to leave braces off for
> single line statements.  That is, this should be:

Sure, should have seem that.

I will make up another patch and resend it.

Ciao
Max
-- 
Träume nicht von Dein Leben: Lebe Deinen Traum!