| Message ID | 1457622051-30189-2-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 10, 2016 at 3:00 PM, Juergen Gross <jgross@suse.com> wrote: > libxl__need_xenpv_qemu() is called with configuration data for console, > vfbs, disks and channels today in order to evaluate the need for > starting a device model for a pv domain. > > The console data is local to the caller and setup in a way to never > require a device model. All other data is taken from the domain config > structure. > > In order to support other device backends via qemu change the interface > of libxl__need_xenpv_qemu() to take the domain config structure as > input instead of the single device arrays. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > tools/libxl/libxl_create.c | 9 +------ > tools/libxl/libxl_dm.c | 59 ++++++++++++-------------------------------- > tools/libxl/libxl_internal.h | 5 +--- > 3 files changed, 18 insertions(+), 55 deletions(-) > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 61b5c01..0e2b0a0 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -1304,7 +1304,6 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, > } > case LIBXL_DOMAIN_TYPE_PV: > { > - int need_qemu = 0; > libxl__device_console console; > libxl__device device; > > @@ -1314,17 +1313,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, > } > > init_console_info(gc, &console, 0); > - > - need_qemu = libxl__need_xenpv_qemu(gc, 1, &console, > - d_config->num_vfbs, d_config->vfbs, > - d_config->num_disks, &d_config->disks[0], > - d_config->num_channels, &d_config->channels[0]); > - > console.backend_domid = state->console_domid; > libxl__device_console_add(gc, domid, &console, state, &device); > libxl__device_console_dispose(&console); > > - if (need_qemu) { > + if (libxl__need_xenpv_qemu(gc, d_config)) { > dcs->dmss.dm.guest_domid = domid; > libxl__spawn_local_dm(egc, &dcs->dmss.dm); > return; > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 4aca38e..4b840f4 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -2107,61 +2107,34 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid) > GCSPRINTF("/local/domain/%d/image/device-model-pid", domid)); > } > > -int libxl__need_xenpv_qemu(libxl__gc *gc, > - int nr_consoles, libxl__device_console *consoles, > - int nr_vfbs, libxl_device_vfb *vfbs, > - int nr_disks, libxl_device_disk *disks, > - int nr_channels, libxl_device_channel *channels) > +int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config) > { > - int i, ret = 0; > + int i, ret; > uint32_t domid; > > - /* > - * qemu is required in order to support 2 or more consoles. So switch all > - * backends to qemu if this is the case > - */ > - if (nr_consoles > 1) { > - for (i = 0; i < nr_consoles; i++) > - consoles[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU; > + ret = libxl__get_domid(gc, &domid); > + if (ret) goto out; Doesn't this mean that if libxl__get_domid() fails, libxl__need_xenpv_qemu() will effectively return "true"? I realize this is a bug in the existing code but there's no need to be bug-for-bug compatible here. :-) Maybe add an 'rc' variable, and make this: rc = libxl__get_domid(); if(rc) goto out; Other than that, looks good to me. -George
On 17/03/16 17:53, George Dunlap wrote: > On Thu, Mar 10, 2016 at 3:00 PM, Juergen Gross <jgross@suse.com> wrote: >> libxl__need_xenpv_qemu() is called with configuration data for console, >> vfbs, disks and channels today in order to evaluate the need for >> starting a device model for a pv domain. >> >> The console data is local to the caller and setup in a way to never >> require a device model. All other data is taken from the domain config >> structure. >> >> In order to support other device backends via qemu change the interface >> of libxl__need_xenpv_qemu() to take the domain config structure as >> input instead of the single device arrays. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> tools/libxl/libxl_create.c | 9 +------ >> tools/libxl/libxl_dm.c | 59 ++++++++++++-------------------------------- >> tools/libxl/libxl_internal.h | 5 +--- >> 3 files changed, 18 insertions(+), 55 deletions(-) >> >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c >> index 61b5c01..0e2b0a0 100644 >> --- a/tools/libxl/libxl_create.c >> +++ b/tools/libxl/libxl_create.c >> @@ -1304,7 +1304,6 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, >> } >> case LIBXL_DOMAIN_TYPE_PV: >> { >> - int need_qemu = 0; >> libxl__device_console console; >> libxl__device device; >> >> @@ -1314,17 +1313,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, >> } >> >> init_console_info(gc, &console, 0); >> - >> - need_qemu = libxl__need_xenpv_qemu(gc, 1, &console, >> - d_config->num_vfbs, d_config->vfbs, >> - d_config->num_disks, &d_config->disks[0], >> - d_config->num_channels, &d_config->channels[0]); >> - >> console.backend_domid = state->console_domid; >> libxl__device_console_add(gc, domid, &console, state, &device); >> libxl__device_console_dispose(&console); >> >> - if (need_qemu) { >> + if (libxl__need_xenpv_qemu(gc, d_config)) { >> dcs->dmss.dm.guest_domid = domid; >> libxl__spawn_local_dm(egc, &dcs->dmss.dm); >> return; >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c >> index 4aca38e..4b840f4 100644 >> --- a/tools/libxl/libxl_dm.c >> +++ b/tools/libxl/libxl_dm.c >> @@ -2107,61 +2107,34 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid) >> GCSPRINTF("/local/domain/%d/image/device-model-pid", domid)); >> } >> >> -int libxl__need_xenpv_qemu(libxl__gc *gc, >> - int nr_consoles, libxl__device_console *consoles, >> - int nr_vfbs, libxl_device_vfb *vfbs, >> - int nr_disks, libxl_device_disk *disks, >> - int nr_channels, libxl_device_channel *channels) >> +int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config) >> { >> - int i, ret = 0; >> + int i, ret; >> uint32_t domid; >> >> - /* >> - * qemu is required in order to support 2 or more consoles. So switch all >> - * backends to qemu if this is the case >> - */ >> - if (nr_consoles > 1) { >> - for (i = 0; i < nr_consoles; i++) >> - consoles[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU; >> + ret = libxl__get_domid(gc, &domid); >> + if (ret) goto out; > > Doesn't this mean that if libxl__get_domid() fails, > libxl__need_xenpv_qemu() will effectively return "true"? > > I realize this is a bug in the existing code but there's no need to be > bug-for-bug compatible here. :-) > > Maybe add an 'rc' variable, and make this: > > rc = libxl__get_domid(); > if(rc) goto out; Okay, will do (next week). > > Other than that, looks good to me. Thanks, Juergen
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 61b5c01..0e2b0a0 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -1304,7 +1304,6 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, } case LIBXL_DOMAIN_TYPE_PV: { - int need_qemu = 0; libxl__device_console console; libxl__device device; @@ -1314,17 +1313,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, } init_console_info(gc, &console, 0); - - need_qemu = libxl__need_xenpv_qemu(gc, 1, &console, - d_config->num_vfbs, d_config->vfbs, - d_config->num_disks, &d_config->disks[0], - d_config->num_channels, &d_config->channels[0]); - console.backend_domid = state->console_domid; libxl__device_console_add(gc, domid, &console, state, &device); libxl__device_console_dispose(&console); - if (need_qemu) { + if (libxl__need_xenpv_qemu(gc, d_config)) { dcs->dmss.dm.guest_domid = domid; libxl__spawn_local_dm(egc, &dcs->dmss.dm); return; diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 4aca38e..4b840f4 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -2107,61 +2107,34 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid) GCSPRINTF("/local/domain/%d/image/device-model-pid", domid)); } -int libxl__need_xenpv_qemu(libxl__gc *gc, - int nr_consoles, libxl__device_console *consoles, - int nr_vfbs, libxl_device_vfb *vfbs, - int nr_disks, libxl_device_disk *disks, - int nr_channels, libxl_device_channel *channels) +int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config) { - int i, ret = 0; + int i, ret; uint32_t domid; - /* - * qemu is required in order to support 2 or more consoles. So switch all - * backends to qemu if this is the case - */ - if (nr_consoles > 1) { - for (i = 0; i < nr_consoles; i++) - consoles[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU; + ret = libxl__get_domid(gc, &domid); + if (ret) goto out; + + if (d_config->num_vfbs > 0) { ret = 1; goto out; } - for (i = 0; i < nr_consoles; i++) { - if (consoles[i].consback == LIBXL__CONSOLE_BACKEND_IOEMU) { + for (i = 0; i < d_config->num_disks; i++) { + if (d_config->disks[i].backend == LIBXL_DISK_BACKEND_QDISK && + d_config->disks[i].backend_domid == domid) { ret = 1; goto out; } } - if (nr_vfbs > 0) { - ret = 1; - goto out; - } - - if (nr_disks > 0) { - ret = libxl__get_domid(gc, &domid); - if (ret) goto out; - for (i = 0; i < nr_disks; i++) { - if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK && - disks[i].backend_domid == domid) { - ret = 1; - goto out; - } - } - } - - if (nr_channels > 0) { - ret = libxl__get_domid(gc, &domid); - if (ret) goto out; - for (i = 0; i < nr_channels; i++) { - if (channels[i].backend_domid == domid) { - /* xenconsoled is limited to the first console only. - Until this restriction is removed we must use qemu for - secondary consoles which includes all channels. */ - ret = 1; - goto out; - } + for (i = 0; i < d_config->num_channels; i++) { + if (d_config->channels[i].backend_domid == domid) { + /* xenconsoled is limited to the first console only. + Until this restriction is removed we must use qemu for + secondary consoles which includes all channels. */ + ret = 1; + goto out; } } diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 345a764..fc7bdab 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1616,10 +1616,7 @@ _hidden int libxl__domain_build(libxl__gc *gc, _hidden const char *libxl__domain_device_model(libxl__gc *gc, const libxl_domain_build_info *info); _hidden int libxl__need_xenpv_qemu(libxl__gc *gc, - int nr_consoles, libxl__device_console *consoles, - int nr_vfbs, libxl_device_vfb *vfbs, - int nr_disks, libxl_device_disk *disks, - int nr_channels, libxl_device_channel *channels); + libxl_domain_config *d_config); /* * This function will fix reserved device memory conflict
libxl__need_xenpv_qemu() is called with configuration data for console, vfbs, disks and channels today in order to evaluate the need for starting a device model for a pv domain. The console data is local to the caller and setup in a way to never require a device model. All other data is taken from the domain config structure. In order to support other device backends via qemu change the interface of libxl__need_xenpv_qemu() to take the domain config structure as input instead of the single device arrays. Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/libxl/libxl_create.c | 9 +------ tools/libxl/libxl_dm.c | 59 ++++++++++++-------------------------------- tools/libxl/libxl_internal.h | 5 +--- 3 files changed, 18 insertions(+), 55 deletions(-)