cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: [DISCUSS]CLOUDSTACK-6191
Date Thu, 17 Jul 2014 07:33:42 GMT
Seems like it could have been fixed with a smaller code replace but I
guess you are right.

@Santosh, Can you split your commit and reapply. At least chipping off
the BridgeVifDriver bit, but preferably all chopped up as other issues
will probably come up.

thanks,

On Wed, Jul 16, 2014 at 6:57 PM, Edison Su <Edison.su@citrix.com> wrote:
> The commit: a600d8408ea86782318139c17cf346c8497943d0, only fixes the "issues" reported
by coverity. Sometimes, coverity may report false alarm, that's what happened in the code
I reverted.
> If we want to make coverity happy, we'd better refactor the code in BridgeVifDriver->Plug
>
>> -----Original Message-----
>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>> Sent: Tuesday, July 15, 2014 4:43 AM
>> To: dev; Edison Su; Santhosh Edukulla
>> Subject: Re: [DISCUSS]CLOUDSTACK-6191
>>
>> Edison,
>>
>> You reverted a600d8408ea86782318139c17cf346c8497943d0 in the end. The
>> code in there is solving a lot of resource problems. Did you pinpoint the exact
>> problem? I can not imagine that there is not a side effect that Santhosh didn't
>> know about and is undesirable that is really the really the root case. We
>> should find that and not just revert because an existing bug was uncovered.
>>
>>
>> On Mon, Jul 14, 2014 at 11:18 PM, Edison Su <Edison.su@citrix.com> wrote:
>> > Yoshikazu fixed the issue related to qemu-img which introduced by his
>> patch in cloudstack-6191.
>> > But there is another issue introduced by commit:
>> a600d8408ea86782318139c17cf346c8497943d0, which causes starting vm
>> failure.
>> > So I checked in a commit: e1095b0110f08fb0c7965f9cf293a6073bbce511, to
>> fix CLOUDSTACK-7051.
>> > So I think, both CLOUDSTACK-6191 and CLOUDSTACK-7051 should be fixed
>> now, no need to revert or change CLOUDSTACK-6191.
>> >
>> >> -----Original Message-----
>> >> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>> >> Sent: Saturday, July 12, 2014 11:02 AM
>> >> To: dev
>> >> Subject: Re: [DISCUSS]CLOUDSTACK-6191
>> >>
>> >> -0 What does it fix and is the solution bonifide. We should fix the
>> >> test suite if it is. The test suite not working is not enough reason
>> >> to revert a commit, it should block the test-suite because the system
>> >> is broken, not because of the way the test suite works.
>> >>
>> >> Disclaimer: I do not know enough of KVM to make a judgement call. I
>> >> just got triggered by the motivation in the mail thread.
>> >>
>> >> On Sat, Jul 12, 2014 at 12:21 AM, Rayees Namathponnan
>> >> <rayees.namathponnan@citrix.com> wrote:
>> >> > +1 Revert now and enable after complete full test in KVM
>> >> >
>> >> > KVM automation blocked more than 7 days due to this defect
>> >> >
>> >> > https://issues.apache.org/jira/browse/CLOUDSTACK-7051
>> >> >
>> >> > Regards,
>> >> > Rayees
>> >> >
>> >> > -----Original Message-----
>> >> > From: Edison Su [mailto:Edison.su@citrix.com]
>> >> > Sent: Friday, July 11, 2014 2:49 PM
>> >> > To: dev@cloudstack.apache.org
>> >> > Subject: [DISCUSS]CLOUDSTACK-6191
>> >> >
>> >> > Move the discussion to the list about CloudStack-6191:
>> >> > Automation test is blocked by this bug, we need to find a solution.
>> >> > My
>> >> suggestion is sort-of-revert-the-patch, but still give admin the
>> >> opportunity to specify the way to optimize KVM volume creation. The
>> reasons:
>> >> >
>> >> > 1.       End user shouldn't care about how the volume is created, is
it
>> >> sparse,flat/thin-provisoned or whatever technology used by
>> >> hypervisor. So we'd better not expose this option in disk offering.
>> >> >
>> >> > 2.        It's true that admin does care about how the volume is created,
so
>> >> we can add a global configuration just for the kvm volume creation.
>> >> For vmware, the option is already there(vmware.create.full.clone to
>> >> control whether link clone or full clone is used to create volume).
>> >> We can add an option, something like kvm.qcow2.volume.create.options.
>> >> > Comments?
>> >>
>> >>
>> >>
>> >> --
>> >> Daan
>>
>>
>>
>> --
>> Daan



-- 
Daan

Mime
View raw message