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

Re: [libvirt] [PATCH] add console support in libxl



Hi, Jim

thanks your reply. one comment below.

 >>>已写入 "Jim Fehlig <jfehlig suse com>"> On 07/04/2013 05:58 AM, Bamvor Jian Zhang wrote:
> > this patch introduce the console api in libxl driver for both pv and 
> > hvm guest.  and import and update the libxlMakeChrdevStr function 
> > which was deleted in commit dfa1e1dd. 
> > 
> > Signed-off-by: Bamvor Jian Zhang <bjzhang suse com> 
> > --- 
> >   src/libxl/libxl_conf.c   |  97 ++++++++++++++++++++++++++++++++++++++ 
> >   src/libxl/libxl_conf.h   |   3 ++ 
> >   src/libxl/libxl_driver.c | 119  
> +++++++++++++++++++++++++++++++++++++++++++++++ 
> >   3 files changed, 219 insertions(+) 
> > 
> > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c 
> > index e170357..08095bc 100644 
> > --- a/src/libxl/libxl_conf.c 
> > +++ b/src/libxl/libxl_conf.c 
> > @@ -332,6 +332,99 @@ error: 
> >   } 
> >    
> >   static int 
> > +libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) 
> > +{ 
> > +    const char *type = virDomainChrTypeToString(def->source.type); 
> > +    int ret; 
> > + 
> > +    if (!type) { 
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, 
> > +                       "%s", _("unexpected chr device type")); 
> > +        return -1; 
> > +    } 
> > + 
> > +    switch (def->source.type) { 
> > +        case VIR_DOMAIN_CHR_TYPE_NULL: 
> > +        case VIR_DOMAIN_CHR_TYPE_STDIO: 
> > +        case VIR_DOMAIN_CHR_TYPE_VC: 
> > +        case VIR_DOMAIN_CHR_TYPE_PTY: 
> > +            if (virAsprintf(buf, "%s", type) < 0) { 
> > +                virReportOOMError(); 
>  
> This will need rebased now that Michal's "Introduce OOM reporting to  
> virAsprintf" patchset is committed. 
>  
> https://www.redhat.com/archives/libvir-list/2013-July/msg00506.html 
>  
> > +                return -1; 
> > +            } 
> > +            break; 
> > + 
> > +        case VIR_DOMAIN_CHR_TYPE_FILE: 
> > +        case VIR_DOMAIN_CHR_TYPE_PIPE: 
> > +            if (virAsprintf(buf, "%s:%s", type, 
> > +                            def->source.data.file.path) < 0) { 
> > +                virReportOOMError(); 
> > +                return -1; 
> > +            } 
> > +            break; 
> > + 
> > +        case VIR_DOMAIN_CHR_TYPE_DEV: 
> > +            if (virAsprintf(buf, "%s", def->source.data.file.path) < 0) { 
> > +                virReportOOMError(); 
> > +                return -1; 
> > +            } 
> > +            break; 
> > +        case VIR_DOMAIN_CHR_TYPE_UDP: { 
> > +            const char *connectHost = def->source.data.udp.connectHost; 
> > +            const char *bindHost = def->source.data.udp.bindHost; 
> > +            const char *bindService  = def->source.data.udp.bindService; 
> > + 
> > +            if (connectHost == NULL) 
> > +                connectHost = ""; 
> > +            if (bindHost == NULL) 
> > +                bindHost = ""; 
> > +            if (bindService == NULL) 
> > +                bindService = "0"; 
> > + 
> > +            ret = virAsprintf(buf, "udp:%s:%s %s:%s", 
> > +                              connectHost, 
> > +                              def->source.data.udp.connectService, 
> > +                              bindHost, 
> > +                              bindService); 
> > +            if ( ret < 0) { 
>  
> Extra space between '(' and 'ret', caught by 'make syntax-check'. 
>  
> > +                virReportOOMError(); 
> > +                return -1; 
> > +            } 
> > +            break; 
> > +        } 
> > +        case VIR_DOMAIN_CHR_TYPE_TCP: 
> > +            if (def->source.data.tcp.protocol ==  
> VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) { 
> > +                ret = virAsprintf(buf, "telnet:%s:%s%s", 
> > +                                  def->source.data.tcp.host, 
> > +                                  def->source.data.tcp.service, 
> > +                                  def->source.data.tcp.listen ?  
> ",server,nowait" : ""); 
> > +            } else { 
> > +                ret = virAsprintf(buf, "tcp:%s:%s%s", 
> > +                                  def->source.data.tcp.host, 
> > +                                  def->source.data.tcp.service, 
> > +                                  def->source.data.tcp.listen ?  
> ",server,nowait" : ""); 
> > +            } 
> > +            if ( ret < 0) { 
>  
> Extra space here too. 
>  
> > +                virReportOOMError(); 
> > +                return -1; 
> > +            } 
> > +            break; 
> > + 
> > +        case VIR_DOMAIN_CHR_TYPE_UNIX: 
> > +            ret = virAsprintf(buf, "unix:%s%s", 
> > +                              def->source.data.nix.path, 
> > +                              def->source.data.nix.listen ? ",server,nowait"  
> : ""); 
> > +            if ( ret < 0) { 
>  
> And here. 
>
> BTW, do all of these types work with Xen?  I've only tested with type 'pty',  
>  
> which works fine with both pv and hvm guests. 
i only test the pty type too. lots of type is introduced in 2b84e445(Add libxenlight driver), i add the missing type compare with qemu driver. maybe it will be used in future. for now, only pty is needed for my console patch.

thanks

Bamvor
> > +                virReportOOMError(); 
> > +                return -1; 
> > +            } 
> > +            break; 
> > +    } 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +static int 
> >   libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) 
> >   { 
> >       libxl_domain_build_info *b_info = &d_config->b_info; 
> > @@ -404,6 +497,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,  
> libxl_domain_config *d_config) 
> >           if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0) 
> >               goto error; 
> >    
> > +        if (def->nserials && 
> > +            (libxlMakeChrdevStr(def->serials[0], &b_info->u.hvm.serial) < 0)) 
> > +            goto error; 
> > + 
> >           /* 
> >            * The following comment and calculation were taken directly from 
> >            * libxenlight's internal function  
> libxl_get_required_shadow_memory(): 
> > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h 
> > index 2b4a281..861d689 100644 
> > --- a/src/libxl/libxl_conf.h 
> > +++ b/src/libxl/libxl_conf.h 
> > @@ -34,6 +34,7 @@ 
> >   # include "configmake.h" 
> >   # include "virportallocator.h" 
> >   # include "virobject.h" 
> > +# include "virchrdev.h" 
> >    
> >    
> >   # define LIBXL_VNC_PORT_MIN  5900 
> > @@ -94,6 +95,8 @@ struct _libxlDomainObjPrivate { 
> >    
> >       /* per domain libxl ctx */ 
> >       libxl_ctx *ctx; 
> > +    /* console */ 
> > +    virChrdevsPtr devs; 
> >       libxl_evgen_domain_death *deathW; 
> >    
> >       /* list of libxl timeout registrations */ 
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c 
> > index 1bae3d6..b8c6832 100644 
> > --- a/src/libxl/libxl_driver.c 
> > +++ b/src/libxl/libxl_driver.c 
> > @@ -420,6 +420,9 @@ libxlDomainObjPrivateAlloc(void) 
> >    
> >       libxl_osevent_register_hooks(priv->ctx, &libxl_event_callbacks, priv); 
> >    
> > +    if (!(priv->devs = virChrdevAlloc())) 
> > +        return NULL; 
> > + 
> >       return priv; 
> >   } 
> >    
> > @@ -432,6 +435,7 @@ libxlDomainObjPrivateDispose(void *obj) 
> >           libxl_evdisable_domain_death(priv->ctx, priv->deathW); 
> >    
> >       libxl_ctx_free(priv->ctx); 
> > +    virChrdevFree(priv->devs); 
> >   } 
> >    
> >   static void 
> > @@ -4518,6 +4522,120 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom,  
> virTypedParameterPtr params, 
> >       return libxlDomainSetSchedulerParametersFlags(dom, params, nparams,  
> 0); 
> >   } 
> >    
> > + 
> > +static int 
> > +libxlDomainOpenConsole(virDomainPtr dom, 
> > +                       const char *dev_name, 
> > +                       virStreamPtr st, 
> > +                       unsigned int flags) 
> > +{ 
> > +    libxlDriverPrivatePtr driver = dom->conn->privateData; 
> > +    virDomainObjPtr vm = NULL; 
> > +    int ret = -1; 
> > +    int i; 
>  
> Needs to be size_t following Daniel's patchset to "Santize iterator variable  
>  
> names & data types" 
>  
> https://www.redhat.com/archives/libvir-list/2013-July/msg00397.html 
>  
> > +    virDomainChrDefPtr chr = NULL; 
> > +    libxlDomainObjPrivatePtr priv; 
> > +    char *console = NULL; 
> > +    int num = 0; 
> > +    libxl_console_type type; 
> > + 
> > +    virCheckFlags(VIR_DOMAIN_CONSOLE_SAFE | 
> > +                  VIR_DOMAIN_CONSOLE_FORCE, -1); 
> > + 
> > +    libxlDriverLock(driver); 
> > +    vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); 
> > +    libxlDriverUnlock(driver); 
> > +    if (!vm) { 
> > +        char uuidstr[VIR_UUID_STRING_BUFLEN]; 
> > +        virUUIDFormat(dom->uuid, uuidstr); 
> > +        virReportError(VIR_ERR_NO_DOMAIN, 
> > +                       _("No domain with matching uuid '%s'"), uuidstr); 
> > +        goto cleanup; 
> > +    } 
> > + 
> > +    if (!virDomainObjIsActive(vm)) { 
> > +        virReportError(VIR_ERR_OPERATION_INVALID, 
> > +                       "%s", _("domain is not running")); 
> > +        goto cleanup; 
> > +    } 
> > + 
> > +    priv = vm->privateData; 
> > + 
> > +    if (dev_name) { 
> > +        for (i = 0 ; !chr && i < vm->def->nconsoles ; i++) { 
>  
> Spaces before the semicolons causes 'make syntax-check' failure. 
>  
> > +            if (vm->def->consoles[i]->info.alias && 
> > +                STREQ(dev_name, vm->def->consoles[i]->info.alias)) { 
> > +                chr = vm->def->consoles[i]; 
> > +                num = i; 
> > +            } 
> > +        } 
> > +        for (i = 0 ; !chr && i < vm->def->nserials ; i++) { 
>  
> Same comment here about spaces before semicolon. 
>  
> > +            if (STREQ(dev_name, vm->def->serials[i]->info.alias)) { 
> > +                chr = vm->def->serials[i]; 
> > +                num = i; 
> > +            } 
> > +        } 
> > +        for (i = 0 ; !chr && i < vm->def->nparallels ; i++) { 
>  
> And here. 
>  
> > +            if (STREQ(dev_name, vm->def->parallels[i]->info.alias)) { 
> > +                chr = vm->def->parallels[i]; 
> > +                num = i; 
> > +            } 
> > +        } 
> > +    } else { 
> > +        if (vm->def->nconsoles) 
> > +            chr = vm->def->consoles[0]; 
> > +        else if (vm->def->nserials) 
> > +            chr = vm->def->serials[0]; 
> > +    } 
> > + 
> > +    if (!chr) { 
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, 
> > +                       _("cannot find character device %s"), 
> > +                       NULLSTR(dev_name)); 
> > +        goto cleanup; 
> > +    } 
> > + 
> > +    if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { 
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, 
> > +                       _("character device %s is not using a PTY"), 
> > +                       NULLSTR(dev_name)); 
> > +        goto cleanup; 
> > +    } 
> > + 
> > +    if (dev_name) { 
> > +        if (STREQ(vm->def->os.type, "hvm")) 
> > +            type = LIBXL_CONSOLE_TYPE_SERIAL; 
> > +        else 
> > +            type = LIBXL_CONSOLE_TYPE_PV; 
> > +        ret = libxl_console_get_tty(priv->ctx, vm->def->id, num, type, &console); 
> > +    } else { 
> > +        ret = libxl_primary_console_get_tty(priv->ctx, vm->def->id, &console); 
> > +    } 
> > +    if ( ret ) 
>  
> Extra spaces here causing 'make syntax-check' failure as well. 
>  
> Regards, 
> Jim 
>  
> > +        goto cleanup; 
> > + 
> > +    if (VIR_STRDUP(chr->source.data.file.path, console) < 0) 
> > +        goto cleanup; 
> > + 
> > +    /* handle mutually exclusive access to console devices */ 
> > +    ret = virChrdevOpen(priv->devs, 
> > +                         &chr->source, 
> > +                         st, 
> > +                         (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); 
> > + 
> > +    if (ret == 1) { 
> > +        virReportError(VIR_ERR_OPERATION_FAILED, "%s", 
> > +                       _("Active console session exists for this  
> domain")); 
> > +        ret = -1; 
> > +    } 
> > + 
> > +cleanup: 
> > +    VIR_FREE(console); 
> > +    if (vm) 
> > +        virObjectUnlock(vm); 
> > +    return ret; 
> > +} 
> > + 
> >   static int 
> >   libxlDomainIsActive(virDomainPtr dom) 
> >   { 
> > @@ -4739,6 +4857,7 @@ static virDriver libxlDriver = { 
> >       .domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */ 
> >       .domainHasManagedSaveImage = libxlDomainHasManagedSaveImage, /* 0.9.2  
> */ 
> >       .domainManagedSaveRemove = libxlDomainManagedSaveRemove, /* 0.9.2 */ 
> > +    .domainOpenConsole = libxlDomainOpenConsole, /* 1.0.8 */ 
> >       .domainIsActive = libxlDomainIsActive, /* 0.9.0 */ 
> >       .domainIsPersistent = libxlDomainIsPersistent, /* 0.9.0 */ 
> >       .domainIsUpdated = libxlDomainIsUpdated, /* 0.9.0 */ 
>  
>  


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