incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Angus <blan...@betterservers.com>
Subject Re: Review Request: API commands to add or remove NIC from a VM
Date Sat, 26 Jan 2013 00:03:32 GMT
My answers inline...

On 01/24/2013 06:49 PM, Marcus Sorensen wrote:
> Brian is probably the right guy to answer some of these, but I'll chime in
> on a few.
>
> On Thu, Jan 24, 2013 at 5:58 PM, Chiradeep Vittal <
> Chiradeep.Vittal@citrix.com> wrote:
>
>> Thanks for this.
>> A few comments:
>> 1. These mutating operations have to be async and hence the API commands
>> have to inherit from BaseAsyncCmd instead of BaseCmd
>>
>
> We can do that
This change has been pushed to the branch
>
>
>> 2. The functional specification is rather sparse -- I.e., it is not of
>> much use for a QA person to develop a comprehensive set of test cases. I
>> recommend taking a look at
>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Design+Document+Temp
>> late to see what more information one could put in there. For example,
>> edge cases (remove the last nic / exceed nic limit), configuration
>> parameters.
I have added more details to the functional spec.  Probably still needs 
more work...
>> 3. There's todos in the code: either this code is important  and cannot
>> ship without the TODO or is a nice to have. It is hard for someone to
>> figure out which case it is. Regardless if the TODOs are not fixed, they
>> should have bugs
>>
>
> I'll have to revisit the code, but I know at least a few that were
> inherited from the existing code base (things that were borrowed). Also I
> know of one or two that were just notes to myself of things to investigate,
> I thought I had removed those but I'll double check.
Some of these have been addressed I'll work on the ones that still exist.
>
>
>> 4. Questions in my mind:
>> A. Do you update instance metadata during this operation?
>> B. Verify IP address is not already used / in range / etc
>>
>
> We don't assign the IP, we call the same code that creates nics in the
> first place, and the IP is assigned via the same functionality that
> currently exists. If there's a problem with using IPs that are already
> used, it's a problem for deployVirtualMachine as well.
>
>
>> C. If you remove a default nic, what happens?
>>
>
> You can't.
>
>
>> D. Can you remove the last nic?
>>
>
> No, because you have to have a default nic, and you can't remove your
> default.
>
> E. Verify live migration after operation?
>>
>
> This has been done for KVM, could be tested for Xen, but the VM definition
> looks good.
I have tested this on XEN devcloud as well
>
>
>> F. Operation blocks / fails during live migration / HA / start / stop
>>
>
> Brian recently changed some of the code related to this, but previously the
> VM state had to be 'stopped', not 'starting', 'stopping', 'running', or
> 'migrating'.
The API calls will fail if the VM is not in a Running or Stopped state
>
>
>> G. Usage events for remove / add operations
>>
>
> I'm unfamilar with usage events. The usages for new nics are tracked,
> though.
>
>
>> H. Interaction of removeNic with extant LB / PF / Static NAT/ FW rules
>>
>
> This is a good one to verify, does removing a nic orphan a port forwarding
> rule, etc, or does removing the nic clean up the rule?
>
>
>> I. Interaction of addNic / updateNic with PF/Static NAT/Source NAT --
>> e.g., the default route might remain the same inside the VM leading to
>> head scratching as to why a specific PF/NAT rule does not work.
>>
>
> This is a non-issue, in my opinion. It's similar to attaching a volume and
> then wondering why it's not formatted and mounted for us right where we
> wanted it. People have all sorts of varying uses for multiple networks, and
> it's up to them to configure what they intend. It would be good to make a
> note in the documentation though, people should know that we don't have
> much control over their VM's OS config and they need to treat it like any
> other server.
>
>
>>
>> Thanks
>>
>> On 1/24/13 3:45 PM, "Brian Angus" <blangus@betterservers.com> wrote:
>>
>>> The add_remove_nics branch has been updated.  It now allows hot swapping
>>> of NICs and you can now add multiple NICs from the same network.  The
>>> feature spec has been updated as the API calls have changed:
>>>
>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Add+Remove+Networks
>>> +to+VMs
>>>
>>> -Brian
>>> On 01/21/2013 04:34 PM, Brian Angus wrote:
>>>> On 01/21/2013 02:02 PM, Marcus Sorensen wrote:
>>>>> On Mon, Jan 21, 2013 at 12:30 AM, Hari Kannan
>>>>> <hari.kannan@citrix.com>wrote:
>>>>>
>>>>> I think you're right on #1, that if you create a VM via API you can add
>>>>> multiple nics to the same network so long as you're not using a VPC.
I
>>>>> don't know for sure off the top of my head though
>>>>>
>>>> I tested this out and it is correct that you can add multiple NICs with
>>>> the same network using the deployVirtualMachine api call.  When I was
>>>> implementing this I thought cloudstack only allowed one NIC per network
>>>> because the com.cloud.network.NetworkManager.createNicForVm function
>>>> does not create a NIC if there is already a NIC that exists on the
>>>> Network (is that the desired behavior or is it a bug?).
>>>>>
>>>>>> 2. Earlier, Marcus had stated that " It doesn't strictly require
a
>>>>>> reboot,
>>>>>> but it does require that you apply a network config. The nic will
hot
>>>>>> plug
>>>>>> but you either have to reboot or manually trigger a dhcp query "
-
>>>>>> wish to
>>>>>> reconfirm this to be the case - also, can you please provide a short
>>>>>> note
>>>>>> on how to do this, so we can add it to the documentation?
>>>>>>
>>>>>
>>>>> We could give some basic examples, but it's largely going to come down
>>>>> to
>>>>> specific OS, for example with linux, it could change by distribution
>>>>> based
>>>>> on the udev rules, the config file locations, dhcp client differences,
>>>>> etc.
>>>>> It would be up to the admin of the VM guest to know how to set up a
>>>>> network
>>>>> card on their guest OS.
>>>>>
>>>>>
>>>>
>>>>
>>>> Right now the code in the add_remove_nics branch only works when the VM
>>>> is in a Stopped state.  I'm currently working on getting it working
>>>> while the VM is in a Running state.
>>>>
>>>>>> 3. Can you please confirm this is possible for any OS (windows/Linux)?
>>>>>>
>>>>>
>>>>> Worst case is that the OS doesn't recognize the new NIC without a
>>>>> reboot.
>>>>> So either you make them power the VM off always before adding a NIC,
>>>>> or let
>>>>> them try to add the NIC and they can reboot if the OS doesn't support
>>>>> it.
>>>>>
>>>>>
>>>>>> 4. is there any UI component developed for this?
>>>>>>
>>>>>
>>>>> No, I think there is a team who specializes in this. I was thinking the
>>>>> other day that the UI will likely always lag behind the API in
>>>>> features,
>>>>> because new features won't get time to be implemented in UI on each
>>>>> release.
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>


Mime
View raw message