With the interdiff, LGTM!

I'll push the patch.

On Wed, Jun 24, 2015 at 12:59 PM, Lisa Velden <[email protected]> wrote:

> Interdiff:
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index e545445..200abca 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -453,7 +453,7 @@ class LUInstanceCreate(LogicalUnit):
>        nic_name = _ComputeInstanceCommunicationNIC(self.op.instance_name)
>
>        for nic in self.op.nics:
> -        if nic and nic[constants.INIC_NAME] == nic_name:
> +        if nic.get(constants.INIC_NAME, None) == nic_name:
>            break
>        else:
>          self.op.nics.append({constants.INIC_NAME: nic_name,
>
>
> And commit message:
>
> Prevent multiple nics for one instance
>
> Check if a nic name is already in the list of all nics before adding it.
> Expand the instance name before that check to ensure that we are always
> checking for the correct name.
>
> On Wed, Jun 24, 2015 at 12:28 PM, Hrvoje Ribicic <[email protected]> wrote:
>
>> Fix title by doing:
>> s/mulitiple/multiple/
>>
>> On Tue, Jun 23, 2015 at 5:16 PM, 'Lisa Velden' via ganeti-devel <
>> [email protected]> wrote:
>>
>>> Check if a nic name is already in the list of all nics before adding it.
>>> Expand the instance name before that check to ensure that we are always
>>> checking for the correct name.
>>>
>>> Signed-off-by: Lisa Velden <[email protected]>
>>> ---
>>>  lib/cmdlib/instance.py | 32 ++++++++++++++++++--------------
>>>  1 file changed, 18 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
>>> index d5b2c6b..e545445 100644
>>> --- a/lib/cmdlib/instance.py
>>> +++ b/lib/cmdlib/instance.py
>>> @@ -439,15 +439,28 @@ class LUInstanceCreate(LogicalUnit):
>>>        raise errors.OpPrereqError("Cannot do IP address check without a
>>> name"
>>>                                   " check", errors.ECODE_INVAL)
>>>
>>> +    # instance name verification
>>> +    if self.op.name_check:
>>> +      self.hostname = _CheckHostnameSane(self, self.op.instance_name)
>>> +      self.op.instance_name = self.hostname.name
>>> +      # used in CheckPrereq for ip ping check
>>> +      self.check_ip = self.hostname.ip
>>> +    else:
>>> +      self.check_ip = None
>>> +
>>>      # add NIC for instance communication
>>>      if self.op.instance_communication:
>>>        nic_name = _ComputeInstanceCommunicationNIC(self.op.instance_name)
>>>
>>> -      self.op.nics.append({constants.INIC_NAME: nic_name,
>>> -                           constants.INIC_MAC: constants.VALUE_GENERATE,
>>> -                           constants.INIC_IP: constants.NIC_IP_POOL,
>>> -                           constants.INIC_NETWORK:
>>> -
>>>  self.cfg.GetInstanceCommunicationNetwork()})
>>> +      for nic in self.op.nics:
>>> +        if nic and nic[constants.INIC_NAME] == nic_name:
>>>
>>
>> This will not work for most nics - they might have some specified entries
>> (bridge), but not a name.
>> I suggest:
>>
>> nic.get(constants.INIC_NAME, None) == nic_name
>>
>>
>>> +          break
>>> +      else:
>>> +        self.op.nics.append({constants.INIC_NAME: nic_name,
>>> +                             constants.INIC_MAC:
>>> constants.VALUE_GENERATE,
>>> +                             constants.INIC_IP: constants.NIC_IP_POOL,
>>> +                             constants.INIC_NETWORK:
>>> +
>>>  self.cfg.GetInstanceCommunicationNetwork()})
>>>
>>
>>
>>>      # timeouts for unsafe OS installs
>>>      if self.op.helper_startup_timeout is None:
>>> @@ -467,15 +480,6 @@ class LUInstanceCreate(LogicalUnit):
>>>      self._CheckDiskArguments()
>>>      assert self.op.disk_template is not None
>>>
>>> -    # instance name verification
>>> -    if self.op.name_check:
>>> -      self.hostname = _CheckHostnameSane(self, self.op.instance_name)
>>> -      self.op.instance_name = self.hostname.name
>>> -      # used in CheckPrereq for ip ping check
>>> -      self.check_ip = self.hostname.ip
>>> -    else:
>>> -      self.check_ip = None
>>> -
>>>      # file storage checks
>>>      if (self.op.file_driver and
>>>          not self.op.file_driver in constants.FILE_DRIVER):
>>> --
>>> 2.4.3.573.g4eafbef
>>>
>>>
>> Hrvoje Ribicic
>> Ganeti Engineering
>> Google Germany GmbH
>> Dienerstr. 12, 80331, München
>>
>> Geschäftsführer: Graham Law, Christine Elizabeth Flores
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
>>
>
>
>
> --
> Lisa Velden
> Software Engineer
> [email protected]
>
> Google Germany GmbH
> Dienerstraße 12
> 80331 München
>
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
>

Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Geschäftsführer: Graham Law, Christine Elizabeth Flores
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Reply via email to