cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <>
Subject [jira] [Commented] (CLOUDSTACK-9539) Support changing Service offering for instance with VM Snapshots
Date Tue, 07 Feb 2017 21:11:41 GMT


ASF GitHub Bot commented on CLOUDSTACK-9539:

Github user rafaelweingartner commented on the issue:
    Long time I do not review a PR here. I have to say, @nvazquez , great job!!
    @serg38 you read my mind!! I had just started reading @nvazquez PR description when you
called me.
    Well documented methods. The methods are short and concise. The code is clear and looks
nice (at least for me).
    I jumped over the 70+ messages here. So, excuse me if I ask something that has already
been clarified before.
    However, as always I have something to complain; I might be getting old, that is probably
why :) . Just teasing; it is more like a set of suggestion;
    * On VMSnapshotVO, what about declaring the instance attribute you create as private?
I know the others are not, and at the end of the day, it does not change much. However, as
long as we are using Java and if we consider “VMSnapshotVO” as a POJO, then it feels a
good idea to use private attributes that are only accessible to their gets/sets.
    * On VMSnapshotManagerImpl, here we have a “service” layer object/component. I would
have the same suggestion regarding the use of private/protected attributes here (this is cosmetics).
    * Still on VMSnapshotManagerImpl, what about extracting the code from lines 358-373 to
a method? This may enable you to write unit test cases for these lines.
    * Also on VMSnapshotManagerImpl, methods “addSupportForCustomServiceOffering”, “updateUserVmServiceOffering”,
“getVmMapDetails”, “changeUserVmServiceOffering”, revertUserVmDetailsFromVmSnapshot,
and “ugradeUserVmServiceOffering” are clean and well documented. However, I am missing
their test cases. What about some test cases here, then it would be awesome.
    * on VMSnapshotManagerImpl, the method “ugradeUserVmServiceOffering” seems to have
a typo mistake ;)
    * the method “revertUserVmDetailsFromVmSnapshot” receives a parameter “userVm”,
but it does not seem to be used.
    I am assuming you have executed the functional test “” and it
passed successfully, right?
    Very good feature to be added. Thanks for the valuable contribution to ACS 👍 

> Support changing Service offering for instance with VM Snapshots
> ----------------------------------------------------------------
>                 Key: CLOUDSTACK-9539
>                 URL:
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the default.) 
>            Reporter: Nicolas Vazquez
>            Assignee: Nicolas Vazquez
> h3. Actual behaviour
> CloudStack doesn't support changing service offering for vm instances which have vm snapshots,
they should be removed before changing service offering.
> h3. Goal
> Extend actual behaviour by supporting changing service offering for vms which have vm
snapshots. In that case, previously taken snapshots (if reverted) should use previous service
offering, future snapshots should use the newest.
> h3. Proposed solution:
> 1. Adding {{service_offering_id}} column on {{vm_snapshots}} table: This way snapshot
can be reverted to original state even though service offering can be changed for vm instance.
> NOTE: Existing vm snapshots are populated on update script by {{UPDATE vm_snapshots s
JOIN vm_instance v ON = s.vm_id SET s.service_offering_id = v.service_offering_id;}}
> 2. New vm snapshots will use instance vm service offering id as {{service_offering_id}}
> 3. Revert to vm snapshots should use vm snapshot's {{service_offering_id}} value.
> h3. Example use case:
> - Deploy vm using service offering A
> - Take vm snapshot -> snap1 (service offering A)
> - Stop vm
> - Change vm service offering to B
> - Revert to VM snapshot snap 1
> - Start vm
> It is expected that vm has service offering A after last step

This message was sent by Atlassian JIRA

View raw message