cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chiradeep Vittal <>
Subject Re: Review Request: API commands to add or remove NIC from a VM
Date Fri, 25 Jan 2013 00:58:43 GMT
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
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
late to see what more information one could put in there. For example,
edge cases (remove the last nic / exceed nic limit), configuration
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
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
C. If you remove a default nic, what happens?
D. Can you remove the last nic?
E. Verify live migration after operation?
F. Operation blocks / fails during live migration / HA / start / stop
G. Usage events for remove / add operations
H. Interaction of removeNic with extant LB / PF / Static NAT/ FW rules
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.


On 1/24/13 3:45 PM, "Brian Angus" <> 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:
>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
>>> <>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 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
>>> 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
>>> 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
>>>> 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
>>> because new features won't get time to be implemented in UI on each
>>> release.

View raw message