Monday, June 17, 2013

Google Summer of Code 2013 Progress


 __      __               __      ____ 
/  \    /  \ ____   ____ |  | __ /_   |
\   \/\/   // __ \_/ __ \|  |/ /  |   |
 \        /\  ___/\  ___/|    <   |   |
  \__/\  /  \___  >\___  >__|_ \  |___|
       \/       \/     \/     \/       
Libvirt [Task]: Successfully build libvirt from source code (rev: v1.0.6-79-g847e1cd ) and run qemu-qeust agent.
To Compile:
$ git clone git://libvirt.org/libvirt.git
$ cd libvirt
$ ./autogen --system
$ make
Start the libvirt daemon:
$ ./daemon/libvitd -d
Make a f18 VM via virt-manager
$ ./tools/virsh
virsh # connect qemu:///system
virsh # start f18
ssh into the VM and install qemu-guest-agent
$ yum install -y qemu-guest-agent ; reboot
Make sure that qemu-guest-agent is running in the guest
$ ps -C qemu-ga # should return pid of the process
If not, then type the command:
$ qemu-ga -d
Return to host and start querying!
$ ./tools/virsh
virsh # qemu-agent-command f18 '{"execute":"guest-network-get-interfaces"}'
References:
http://aglitke.wordpress.com/2011/02/24/how-to-hack-on-a-local-copy-of-libvirt/
http://wiki.libvirt.org/page/Qemu_guest_agent
http://wiki.qemu.org/Features/QAPI/GuestAgent#Example_usage


Git: Set up the git repository: https://github.com/nehaljwani/gsoc2013-libvirt
Steps:
Make the git repo via web interface on github.com
Clone the repo:
$ git clone git://libvirt.org/libvirt.git
$ git remote add github git@github.com:nehaljwani/gsoc2013-libvirt.git
$ git push github master
To reflect changes from main repo into your repo
$ git fetch origin
$ git push github origin/master:master
To reflect local changes into your repo
$ git push github master
While building, if you get the error:
Unable to find current revision in submodule path '.gnulib'
Then do:
$ rm -fr .gnulib
$ git submodule update --init

 __      __               __     ________  
/  \    /  \ ____   ____ |  | __ \_____  \ 
\   \/\/   // __ \_/ __ \|  |/ /  /  ____/ 
 \        /\  ___/\  ___/|    <  /       \ 
  \__/\  /  \___  >\___  >__|_ \ \_______ \
       \/       \/     \/     \/         \/
Libvirt [Task]: Setting up the qemu-guest-agent
To be able to use GA users needs to create virtio serial port with special name org.qemu.guest_agent.0. In other words, one needs to add this to his/her domain XML under <devices>:
$ virsh edit <domain-name>
<channel type='unix'>
   <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/>
   <target type='virtio' name='org.qemu.guest_agent.0'/>
</channel>

Libvirt [Update]:
        * Discussion on why virTypedParams > struct/XML: Upper layer app doesn't have to parse the returned xml string, more extensible.
        * Update Michal's patches (Struct and XML) and modify them with virTypedParameter.
        * Asked Michal to send his XML patches. (The one on list was incomplete).

Libvirt [Task]: Analyze previous patches submitted by Michal Privoznik, Re-factor the struct patch (to make it compatible with the latest version of libvirt) and understand the implementation.
Updated struct patch: https://gist.github.com/nehaljwani/5829005

 __      __               __     ________  
/  \    /  \ ____   ____ |  | __ \_____  \ 
\   \/\/   // __ \_/ __ \|  |/ /   _(__  < 
 \        /\  ___/\  ___/|    <   /       \
  \__/\  /  \___  >\___  >__|_ \ /______  /
       \/       \/     \/     \/        \/ 
Git: Setting up git branch
$ git checkout -t master -b workbranch
After making changes:
$ git push github

Libvirt [Update]: Main points in conversation with eblake:
        * If virTypedParamenterPtr * is to be used, it is not advisable to name it as virTypedParameterPtrPtr
        * If at all a 2D pointer is required to be used, take virDomainGetCPUStats as example.
        * It is required that new APIs should be more user-friendly, [usability feature] and return allocated answers, rather than requiring
          the user to pre-allocate
                e.g. implementation :  virConnectListAllDomains (It also has a 3D pointer implementation)
        * Before working on implementation, it is advised to make sure that all have agreement on the upstream list that the interface is sane
          Therefore, I'll be better off proposing the function signature and documentation comments to the list, to get feedback if my proposed
          API makes sense for usability
        * With reespect to the API, we definitely want a 2d result, but it is not finalized whether it will be easier to document a return where
          the user looks at array[a*n+b] or array[a][b] (where a and b are the iterators, and n is the number of parameters per interface).
          Depending on that answer, we either document the public API interface as virTypedParameterPtr *params (allocate a 1d array with
          embedded 2d information) or virTypedParameterPtr **params (allocate a 2d array)
        * Whenever VIR_TYPED_PARAM_STRING is used, it should always be strdup'd using the helper functions in src/util/virtypedparam.h
          Since virTypedParameterAssign doesn't do it, virTypedParamsAddString should be used.
        * Although we want the feature, reasons for not accepting Michal's Patch:
                -  Because the API design wasn't complete
                -  We weren't willing to add an API that wasn't extensible
        * Purpose of using virTypedParameter: It provides us extensibility - as long as we tell the user how many interfaces and how many
          parameters per interface, then we can add more parameters per interface
        * Introduction to new acl functionality: ACLs are access control lists that allow fine-grained access controls on what APIs
          individual users can use. For instance, you can state that user 'foo' can only see a subset of all domains managed by libvirt,
          rather than having global access to the entire list (Introduced by Daniel)

Libvirt [Task]:  Prepare RFC for API
 __      __               __        _____  
/  \    /  \ ____   ____ |  | __   /  |  | 
\   \/\/   // __ \_/ __ \|  |/ /  /   |  |_
 \        /\  ___/\  ___/|    <  /    ^   /
  \__/\  /  \___  >\___  >__|_ \ \____   | 
       \/       \/     \/     \/      |__| 
Libvirt [Update]: Discussion regarding updating libvirt.org/api_extension.html with eblake. It is outdated. Doesn't work with current libvirt version.

Libvirt [Update]: Main points relating to RFC on the list: [http://www.mail-archive.com/libvir-list@redhat.com/msg79793.html]
        * 2 versions of API proposed: Using 1D virTypedParams and 2D virTypedParams.
        * We find that using such structures will increase complexity. Using fixed structs/xml is more simple. Main trouble is caused by
          multiple addresses per interface.
        * Suggestions for method-name : dhcp|snoop|agent
        * Unlikeliness of extension and ease of use super power extensibility. Hence we move back to structs

Libvirt [Update]: Findings w.r.t method 3:
        * nwfilter_dhcpsnoop.c and nwfilter_learnipaddr.c contain parts for snooping. Some methods are explored.
        * Its a pre-requisite to pass the MAC address beforehand.
        * virNWFilterLearnIPAddress is the method which we are interested in. It is indirectly used by virDomainAttachDevice.
        * Discussion halted. Phase 1 is to be completed first.

Libvirt [Update]:
        * Patch using virTypedParams completely discarded. Shifting back to structs.
        * Creating new patch to work with libvirt version 1.1.1

Git: Revoking commits, solving errors
To revoke a recent commit (not pushed), do:
$ git reset --soft HEAD~1
If you are working on a remote branch, and want to update it to the latest version (removing all your changes):
$ git checkout .
$ git pull origin master
$ git reset --hard origin/master
If you still get the error: "your branch is ahead of 'origin/master' by x commits", do:
$ git fetch
Libvirt [Task]: Patch to be updated as per reviews provided by mentor.

Git: Config for git send-email with gmail id
$ git config --global sendemail.smtpserver smtp.gmail.com
$ git config --global sendemail.smtpserverport 587
$ git config --global sendemail.smtpencryption tls
$ git config --global sendemail.smtpuser nehaljw.kkd1@gmail.com

Libvirt: Before sending any patches, make sure to do:
  ./configure --enable-werror
and run the tests:
  make check
  make syntax-check
  make -C tests valgrind


 __      __               __      .________
/  \    /  \ ____   ____ |  | __  |   ____/
\   \/\/   // __ \_/ __ \|  |/ /  |____  \ 
 \        /\  ___/\  ___/|    <   /       \
  \__/\  /  \___  >\___  >__|_ \ /______  /
       \/       \/     \/     \/        \/ 
Git: Sending multiple patches via email:
$ git format patch HEAD~5 
$ git send-email --cover-letter --no-chain-reply-to --annotate --to=jyang@redhat.com --cc=nehaljw.kkd1@gmail.com
OR
$ git send-email -5 --cover-letter --no-chain-reply-to --annotate --to=jyang@redhat.com --cc=nehaljw.kkd1@gmail.com
There seems to be a bug in git, hence, after the above command, instead of letting me edit the emails, git opens the cover letter twice, and doesn't let me edit the other emails. Hence, after editing the cover letter, you'll find that the patches are located in /tmp/<some-random-folder>/, and one can edit them.

Libvirt [Task]: Patch to be updated as per reviews provided by mentor.

Libvirt [Update]:Findings w.r.t method 2:
        * bridge_driver.c is contains function networkDnsmasqLeaseFileNameDefault (retrieves driverState->dnsmasqStateDir)
          which returns location of leases file for a given virtual network
        * Experiments caried out to analyze the format of leases file, using 2 VMs and 3 virtual networks.
        * According to laine, the problem is that some day libvirt wants all of the network stuff to live in a separate daemon,
          and creating a small API to return the value of driverState->dnsmasqStateDir in the driver would instead make
          them more closely bound. Also, nwfilter can snoop for dynamically alotted as well as statically alotted ip addresses. Hence, using
          leases file is not altogether a best idea.
        * Idea is to make the API more generalized, and not for specific purpose. It should use typed parameters
          (e.g. parameter fields including VIR_NETWORK_CONFIG_DIR, VIR_NETWORK_LEASE_FILE, etc).
        * To be posted as RFC on the list.

Libvirt [Task]: Prepare RFC for Introducing API to query configurations directories.

 __      __               __       ________
/  \    /  \ ____   ____ |  | __  /  _____/
\   \/\/   // __ \_/ __ \|  |/ / /   __  \ 
 \        /\  ___/\  ___/|    <  \  |__\  \
  \__/\  /  \___  >\___  >__|_ \  \_____  /
       \/       \/     \/     \/        \/ 
Libvirt [Update]: Why API to query configurations directories doesn't exist:
        *That is internal to the network driver. Under normal circumstances there should be no reason for
          anything external to know that. Libvirt doesn't want them to rely on information that
                   1) is specific to a particular implementation of the back-end of the network driver and
                   2) may change in the future, causing their code to not work.
        *It's the same reason you would want to hide as much of the internal implementation of any piece of
         software - the more you reveal in an official API, the more you have to maintain *forever*, which is
         especially difficult if it becomes deprecated/irrelevant.

Libvirt [Task]: To continue with RFC anyway.
Libvirt [Task]: Final review of method 1 implementation complete. Patch sent to the list [https://www.redhat.com/archives/libvir-list/2013-July/msg01553.html]

Libvirt [Task]: Post RFC of method 2 on list:
Libvirt [Task]: Start working on method 2 assuming that I have access to the leases file using XYZ API

 __      __               __     _________ 
/  \    /  \ ____   ____ |  | __ \______  \
\   \/\/   // __ \_/ __ \|  |/ /     /    /
 \        /\  ___/\  ___/|    <     /    / 
  \__/\  /  \___  >\___  >__|_ \   /____/  
       \/       \/     \/     \/           
Libvirt [Update]: Method 2 almost complete, only virsh support remaining.

No comments: