Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (athena.apache.org: domain of williamstevens@gmail.com
 designates 209.85.220.46 as permitted sender)
MIME-Version: 1.0
Sender: williamstevens@gmail.com
In-Reply-To: <1484997602.30153.1417509150148.JavaMail.zimbra@li.nux.ro>
References: <BDFB04ED-24C1-452A-80F5-D51277213E2F@schubergphilis.com>
	<CADOmhZwktBnO+MNuAOXp1rJBf5G2c6BM1adfqu7ajk9OVTdvqQ@mail.gmail.com>
	<CAC2pano2uYSwQEeh9n6SrdPHCy5F12s56yxUjmuatiQsL8-uCg@mail.gmail.com>
	<CAC2panqfSFDVTXqdikWG9bN-_k7yb++EGqGQpfEAw=wJabOO_Q@mail.gmail.com>
	<77B337AF224FD84CBF8401947098DD87312F5DFD@SJCPEX01CL01.citrite.net>
	<CAC2panrhfTd4kXyV9FSh_ZXpCzHqjrxS_AfxjeKFnnsrOJbS9w@mail.gmail.com>
	<CANLSSBX=zGbTp21A4qqicJO3xB=DjxQX5TQpWNWJjLpqdAtT=g@mail.gmail.com>
	<1484997602.30153.1417509150148.JavaMail.zimbra@li.nux.ro>
Date: Tue, 2 Dec 2014 10:11:40 -0500
Message-ID: 
 <CAC2panoq92gGp0zZsPj3jYaS44+fdSKw37O7Cub8PAT5jGkYWA@mail.gmail.com>
Subject: Re: 05bec59c - kvm, qcow, systemvm qemu-img -o compat
From: Will Stevens <wstevens@cloudops.com>
To: "Nux!" <nux@li.nux.ro>
Cc: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>,
 Edison Su <Edison.su@citrix.com>,
	Wilder Rodrigues <WRodrigues@schubergphilis.com>,
 Leo Simons <LSimons@schubergphilis.com>
Content-Type: multipart/alternative; boundary=047d7bd75c70f1077805093d2483

--047d7bd75c70f1077805093d2483
Content-Type: text/plain; charset=UTF-8

@Rohit:  If the script is run with 'set -e' it should not be a problem
because I was planning to do an explicit 'set +e' before the call and then
a 'set -e' after the call.  This should handle this case, no?

@Nux: When you say "EL7 comes with qemu-img 1.5.3 and it requires the
compat option", what do you mean by that?  Are you saying that if you run
the build system vms job on EL7, that you have to specify the compat option
even though it comes with qemu-img 1.5.3?  That should not be the case
because the default compat flag should not have been changed till 1.7.  If
you are talking about having a KVM host which runs EL7 and has a qemu-img
version of 1.5.3, then that is not really relevant for this bash script.
The version of qemu-img which this script  checking is the version on the
build system, not on the KVM host.  It is validating if the compat option
needs to be passed to the 'qemu-img convert' command in order for the image
to have the 'compat=0.10' option set.  We are not debating that the
'compat=0.10' option needs to be set, that is for sure.  We are determining
when it needs to be explicitly set on the build systems.

In short...
Jenkins kicks off a job to build the system vms.  This build system could
have one of many different qemu-img versions.  When this script runs on the
build system, we need to determine which command needs to be run in order
to output a 'compat=0.10' compatible image.

I will do some testing with Edison's suggestion and see if I can come up
with a solution that way as well and then we can pick which we want to
implement.

Cheers,

Will


*Will STEVENS*
Lead Developer

*CloudOps* *| *Cloud Solutions Experts
420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
w cloudops.com *|* tw @CloudOps_

On Tue, Dec 2, 2014 at 3:32 AM, Nux! <nux@li.nux.ro> wrote:

> As I said above EL7 comes with qemu-img 1.5.3 and it requires the compat
> option, so it's not only 1.7+.
> Personally I'm in favour of Will's approach.
>
> Lucian
>
> --
> Sent from the Delta quadrant using Borg technology!
>
> Nux!
> www.nux.ro
>
> ----- Original Message -----
> > From: "Rohit Yadav" <bhaisaab@apache.org>
> > To: "Will Stevens" <wstevens@cloudops.com>
> > Cc: "Edison Su" <Edison.su@citrix.com>, "Wilder Rodrigues" <
> WRodrigues@schubergphilis.com>, dev@cloudstack.apache.org,
> > "Leo Simons" <LSimons@schubergphilis.com>
> > Sent: Tuesday, 2 December, 2014 07:36:21
> > Subject: Re: 05bec59c - kvm, qcow, systemvm qemu-img -o compat
>
> > Hi Will,
> >
> > While we can use your first solution that checks exit status ($?) but in
> > case some is running the script with a "set -e", the script will simply
> > break when $? is not zero. I think if we know above which qemu-img
> version
> > we should we the compat option (1.7?) than that's great we should we
> that.
> > Thanks for looking into this.
> >
> > Regards.
> >
> > On Tue, Dec 2, 2014 at 8:01 AM, Will Stevens <wstevens@cloudops.com>
> wrote:
> >
> >> Thanks for that. Does that mean that we prefer to do an if else based on
> >> version rather than basically doing a try catch?
> >>
> >> I have not been involved in previous bash scripting for CS, so I am
> >> willing to fall in line with the preferred method. What I proposed was
> >> based on how similar problems were solved in other code in the same
> file.
> >>
> >> If I check for version, do you agree that I should check for 1.7 as the
> >> version since that is the version where the compact default changed?
> >>
> >> I will try to get this resolved tomorrow.
> >>
> >> Cheers,
> >>
> >> Will
> >> On Dec 1, 2014 9:16 PM, "Edison Su" <Edison.su@citrix.com> wrote:
> >>
> >>>  qemu-img |head -n 1|awk '{print $3}'
> >>>
> >>> should show the version of qemu-img
> >>>
> >>>
> >>>
> >>> *From:* williamstevens@gmail.com [mailto:williamstevens@gmail.com] *On
> >>> Behalf Of *Will Stevens
> >>> *Sent:* Monday, December 01, 2014 2:17 PM
> >>> *To:* dev@cloudstack.apache.org
> >>> *Cc:* Leo Simons; Wilder Rodrigues; Rohit Yadav; Edison Su
> >>> *Subject:* Re: 05bec59c - kvm, qcow, systemvm qemu-img -o compat
> >>>
> >>>
> >>>
> >>> Alright, so far I have found the following:
> >>>
> https://github.com/qemu/qemu/commit/9117b47717ad208b12786ce88eacb013f9b3dd1c
> >>>
> >>>
> >>>
> >>> Basically, if the qemu-img version is less than 1.7, we should run the
> >>> non 'compat' option version and if the version is 1.7 or greater, we
> should
> >>> run the new command with the 'compat' version.
> >>>
> >>>
> >>>
> >>> Unfortunately I am not able to find a way to get the qemu-img version
> >>> from the command line.
> >>>
> >>>
> >>>
> >>> I am looking to basically add a conditional to try running with the
> >>> compat option and if that fails, then run it without the compat option.
> >>>
> >>>
> >>>
> >>> Basically, I would be replacing this:
> >>>
> >>>
> >>>
> >>> qemu-img convert -o compat=0.10 -f raw -c -O qcow2 raw.img
> >>> "${appliance_build_name}-kvm.qcow2"
> >>>
> >>>
> >>>
> >>> With this:
> >>>
> >>>
> >>>
> >>> set +e
> >>>
> >>> qemu-img convert -o compat=0.10 -f raw -c -O qcow2 raw.img
> >>> "${appliance_build_name}-kvm.qcow2"
> >>>
> >>> local qemuresult=$?
> >>>
> >>> set -e
> >>>
> >>> if [ ${qemuresult} != 0 ]; then
> >>>
> >>>   log INFO "qemu-img convert failed, trying without compat option"
> >>>
> >>>   qemu-img convert -f raw -c -O qcow2 raw.img
> >>> "${appliance_build_name}-kvm.qcow2"
> >>>
> >>> fi
> >>>
> >>>
> >>>
> >>> What do you guys think?  Is this a good enough solution?  If you guys
> >>> agree I will implement it in master and make sure it works, then we can
> >>> merge the change back to 4.5 to fix that branch as well.
> >>>
> >>>
> >>>
> >>> Let me know if you have issues with this approach...
> >>>
> >>>
> >>>
> >>> Cheers,
> >>>
> >>>
> >>>
> >>> Will
> >>>
> >>>
> >>>
> >>>
> >>> *Will STEVENS*
> >>>
> >>> Lead Developer
> >>>
> >>>
> >>>
> >>> *CloudOps | *Cloud Solutions Experts
> >>>
> >>> 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
> >>>
> >>> w cloudops.com *|* tw @CloudOps_
> >>>
> >>>
> >>>
> >>> On Mon, Dec 1, 2014 at 4:06 PM, Will Stevens <wstevens@cloudops.com>
> >>> wrote:
> >>>
> >>> Edison,
> >>>
> >>> According to this page the default compat option is 0.10:
> >>> http://manpages.ubuntu.com/manpages/trusty/man1/qemu-img.1.html
> >>>
> >>>
> >>>
> >>> Did you find that to not be the case and is that why you had to add the
> >>> compat option?
> >>>
> >>>
> >>>
> >>> BTW, in an attempt to get the master system vms building again, I
> >>> committed a change to master to remove the compat option.  We now have
> >>> master system vms building correctly, but Rohit rightly pointed out
> that I
> >>> had basically reverted your change.
> >>>
> >>>
> >>>
> >>> I created a Jira ticket for this issue:
> >>> https://issues.apache.org/jira/browse/CLOUDSTACK-7959
> >>>
> >>>
> >>>
> >>> I have not reverted my change in master at this point, so it is
> building
> >>> right now, but I also did not make the change to 4.5, so that branch is
> >>> currently failing to build system vms.
> >>>
> >>>
> >>>
> >>> I will see if I can track down in which version of qemu the compat
> option
> >>> was added so we can add some intelligent logic around this command.
> >>>
> >>>
> >>>
> >>> Will
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> *Will STEVENS*
> >>>
> >>> Lead Developer
> >>>
> >>>
> >>>
> >>> *CloudOps | *Cloud Solutions Experts
> >>>
> >>> 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
> >>>
> >>> w cloudops.com *|* tw @CloudOps_
> >>>
> >>>
> >>>
> >>> On Thu, Nov 20, 2014 at 1:07 PM, edison su <sudison@gmail.com> wrote:
> >>>
> >>> Hi Leo,
> >>> Our new internal build system are using ubuntu 14.04 or something like
> >>> that, which has qemu 1.x installed by default, that's why I added the
> >>> "compat" option in the build script, otherwise, the image build by
> >>> qemu 1.x, won't work on RHEL 6.x.
> >>> The fix would be, in the build script, check the version of qemu-img,
> >>> if it's 0.x, then don't add "compat" option. There are a lot of people
> >>> still using RHEL 6.x as KVM hypervisor, we have to make sure the image
> >>> we build can still work on these machines.
> >>>
> >>>
> >>> On Thu, Nov 20, 2014 at 6:49 AM, Leo Simons <
> LSimons@schubergphilis.com>
> >>> wrote:
> >>> > Hey Edison, all,
> >>> >
> >>> >
> >>>
> https://github.com/apache/cloudstack/commit/05bec59c1498dbcfb8a1089c86855fd3b433ea58
> >>> >
> >>> > breaks our internal build with
> >>> >
> >>> >     + qemu-img convert -o compat=0.10 -f raw -c -O qcow2 raw.img \
> >>> >       systemvmtemplate-systemvm-persistent-config-4.5.0.124-kvm.qcow2
> >>> >     Unknown option 'compat'
> >>> >     qemu-img: Invalid options for file format 'qcow2'.
> >>> >
> >>> > This is on CentOS release 6.6, which has
> >>> qemu-img-0.15.0-1.el6.rfx.x86_64.
> >>> >
> >>> > Based on
> >>> >
> >>>
> http://secure-web.cisco.com/1F5zlT8tpWX5PfbJbuzUoso6OWYSnPdIEm5Ue6-gXqJ1a69oLY46nrS2BKoFjZFPlr0DmQwfZHdm2XjfkVhbuMFrET2ilAqQyrI76EYyuxNVnTHaRHH_4bMCtgrtgasDwW1zTJWKDQp8d-HuwyPYokzHAiPFGp7w1CyCabrBpkEw/http%3A%2F%2Fwiki.qemu.org%2FOlderNews
> >>> > that seems like it is a really old qemu (august 2011).
> >>> >
> >>> > I'm guessing you have a newer OS / newer qemu? Can you please let me
> >>> know what OS, OS version and qemu(-img) version you are using?
> >>> >
> >>> > Also, does anyone know if there some minimum version of qemu-img that
> >>> should be used / cloudstack assumes? Is 0.15 still ok to do an
> acceptable
> >>> image conversion with? (we currently don't have any kvm use ourselves,
> but,
> >>> I'd like for our build infra to produce useful kvm images nonetheless).
> >>> >
> >>> > According to
> >>> >
> >>>
> http://secure-web.cisco.com/1pFLbDg2BMTb54NUG5_lYr4mAwFcn9tHP-Weq28Sj5SFqgRKker_DkUKLf6RdwVXHI66uQQNVvD74D6rs6Pc0srpLX0Ejh6nW6o9mBZ-AhQzygMrB_OfUQNLtsB-myd-CA0rJowOhs8ZTsIVr27mgJM4v6ay93D_64JLEBebrlnM/http%3A%2F%2Fwiki.qemu.org%2FChangeLog%2F1.1
> >>> > the -o compat switch was introduced in 1.1.
> >>> >
> >>> > According to
> >>> >
> >>>
> https://github.com/qemu/qemu/commit/9117b47717ad208b12786ce88eacb013f9b3dd1c
> >>> > the default format was changed from 0.10 to 1.1 in qemu 1.7 and
> onwards.
> >>> >
> >>> > The libvirt people
> >>> >   https://bugzilla.redhat.com/show_bug.cgi?id=997977
> >>> > say they pass -o compat when qemu supports it (so when v >= 1.1 I
> >>> imagine).
> >>> >
> >>> > I think we should do the same in the build script and I'll make a
> patch.
> >>> >
> >>> > But, should we publish newer-format images too? According to
> >>> >
> >>>
> http://secure-web.cisco.com/193Tci801hAyVHNqtfHy56-aPW58HHhzwlnmfvPZQnrbtHxURQC0m3kDHxk-PlYYLhCZX0f768ZYK2fV1WbMFpQPz5JfmFt0S8cuzCSrykjOEx9isfBwORZu6I4XdU3jo-WdJcDgtoH-KOwKcGRX5TGZrVva_BDOYDQzF-7QAg8w/http%3A%2F%2Fwiki.qemu.org%2FFeatures%2FQcow3
> >>> > the new format is much better so I imagine qcow/kvm users will really
> >>> appreciate the newer formats.
> >>> >
> >>> > Thoughts?
> >>> >
> >>> >
> >>> > cheers,
> >>> >
> >>> >
> >>> > Leo
> >>> >
> >>>
> >>>
> >>>   --
> >>> Best Regards,
> >>> Edison
> >>>
> >>>
> >>>
> >>>
> >>>
>

--047d7bd75c70f1077805093d2483--