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

Re: [Xen-devel] [PATCH 5/9] libxl: Do root checks once in libxl__domain_get_device_model_uid


  • To: Ian Jackson <Ian.Jackson@xxxxxxxxxx>
  • From: George Dunlap <George.Dunlap@xxxxxxxxxx>
  • Date: Wed, 28 Nov 2018 17:38:58 +0000
  • Accept-language: en-GB, en-US
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>
  • Delivery-date: Wed, 28 Nov 2018 17:39:08 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHUg1AZx0hai2T/YE6tdYH3dYWCMKVlWl0AgAAQugA=
  • Thread-topic: [PATCH 5/9] libxl: Do root checks once in libxl__domain_get_device_model_uid


> On Nov 28, 2018, at 4:39 PM, Ian Jackson <ian.jackson@xxxxxxxxxx> wrote:
> 
> George Dunlap writes ("[PATCH 5/9] libxl: Do root checks once in 
> libxl__domain_get_device_model_uid"):
>> At the moment, we check for equivalence to literal "root" before
>> deciding whether to add the `runas` command-line option to QEMU.  This
>> is unsatisfactory for several reasons.
> 
> I was in two minds about the gotos in the earlier version of this
> patch.  But here they are getting quite out of hand.
> 
> I know that in the hypervisor this kind of thing is tolerated (wrongly
> IMO) but can we please not have it here.

It is a bit strange having to work with one maintianer who thinks a handful of 
simple gotos is an issue, and another maintainer who thinks having switch case 
statements appear in the middle of if() { } blocks is perfectly normal. :-)

> This may mean splitting stuff out into a sub-function.  That could be
> done some time between "Move dm user determination logic into a helper
> function" and this patch I guess.

I’m afraid you’re going to have to give me a bit more guidance here: It’s not 
clear to me what would be split into a sub-function, and how that would make 
the code easier to follow while avoiding unnecessary code duplication.

Do you propose replacing “goto root_check;” with “root_check(); goto out;” in 
all locations?  Or something else?

 -George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.