Re: [Xen-devel] 9p file system for xen
Hello, Wei has suggested I "send a summarised email on [my] work to xen-devel so that people are aware of [my] work." To that end, and to save Wei from reinventing the wheel, I am providing the following: First to correct a misconception, I followed the state change protocol of xen, as outlined by Ian Campbell in his 6/11 email on "Re:[xen-devel]grant tables and driver handshaking". This was tested early in the summer when I passed a small amount of data between my initial front and back ends. The code resides in the files p9_front.c, p9_front_driver.c on the front end and xen_9pif.c on the backend. I am attaching an updated version of a writeup I made this summer. It's table of contents is: 1. The state of the project 2. The approach I took 3. How to build my project 4. How to use it. Sincerely, Linda Jacobson I. PROJECT STATUS The project compiles and builds. The backend initialization works. The front end works partway through initialization. It dies in probe. This is because I initialized the mount path in the backend using the xenstore backend path, and tried to access it in the frontend with its path. Currently this path is hardcoded in the init code in xen_9pif.c. As I wrote in my commit message, this should be done in initialization. Ideally, to handle multiple mounts, the xenstore path should have a "mount directory" that contains all the file system mount paths. In addition, all of the front and backend code to manage the ring and the one entry grant table, as well as transferring data to and from the shared page has been tested and works. Most of the code for the entire project has been written. What I was going to do next (in this order) was: 1. Test: a. That the data transferred accurately. b. That the organization of the data being transferred is as I understand it. 2. Add and test transferring > 1 page of data. If 3b is AIUI, this is trivial. 3. Restore some of the configuration dependencies in the makefiles in qemu, and set them in the configure file. (See notes below) 4. Fix the white space in the different files. II. APPROACH I re-used a fair amount of virtio 9p code to produce the above. In this respect, the front-end required more code, but less modification to existing code. II. A. Front-end When I began, I assumed the code should eventually reside in net/9p. Only one file in this directory trans_virtio.c contains code that is virtio specific. The files client.c and mod.c, while technically part of virtio, could be used as is, and my code calls functions in these files. What I realized, in writing the backend, is that virtio provided the code that takes the initial client request, massages it, and "unmarshals" these requests in the backend. Client.c and mod.c, do this massaging in the front-end. The format of the data that client.c sends in a request is easily moved to a scatter-gather list. p9_front_driver.c contains all the functions specified in the xenbus_driver struct. The module initialization registers the front end with xenbus, and calls the 9p initialization code (in trans_xen9p.c) to register the 9p transport. The functions in trans_xen9p.c mimic their counterparts in trans_virtio.c, in that they provide the same functionality. They are the interface between the functions in client.c and the xen protocol. They receive requests from the client and call functions in p9_front.c to issue the request. The function in trans_xen9p.c, that handles these requests, is specified in the 9p initialization code. p9_front.c contains the code that talks to the back end. It contains the p9_connect function, which is invoked when the front and back-end are connected. It also manages the ring, and the grant table (which, at present has only one entry), receives requests from trans_xen9p.c, transfers the data into a shared page, and issues the request to the backend. When it receives a response, it passes the response information to req_done (in trans_xen9p.c), which processes it, and calls the client callback function. FYI - in linux/fs/fsdev there is the core of the 9p client code. I did not touch this code. II. B. Backend This was more complicated so I'll break it down into Background and What I did. Background: The virtio 9p backend code is composed of many files: virtio-9p.c contains most of the code to interpret the request from the client, and a few virtio-specific functions. virtio-9p-xattr.c and virtio-9p-xattr-user.c handle extended attribute requests. These files reside in qemu/hw/9pfs. virtio-9p-co*.c contain co-routines for all the functions in virtio-9p.c and virtio-xattr.c. virtio-9p-proxy.c, virtio-9p-synth.c, virtio-9p-local.c implement the various file systems that virtio supports. virtio-9p-device.c contains an implementation of the qemu object mod
Re: [Xen-devel] 9p file system for xen
On 12/1/2015 7:46 AM, Wei Liu wrote: On Tue, Dec 01, 2015 at 07:37:43AM -0700, Linda wrote: On 12/1/2015 4:47 AM, Wei Liu wrote: On Mon, Nov 30, 2015 at 12:19:18PM -0500, Neil Sikka wrote: Hi Wei, could you please explain why/how you would have done the project differently now and why these patches are not "good"? From my conversation with Linda, I understood that her code is "Independent of virtio except the 9pvirtio specific code, which is used extensively." I need to implement a xen transport for 9pfs. Linda was essentially doing the same. But she didn't specify the canonical protocol between frontend and backend. For my own edification: In the interests of the limited time of my internship, we decided I shouldn't do the initialization using the xen toolstack. Were there are other expediencies that I'm unaware of? It's not about toolstack. Toolstack merely sets up xenstore nodes according to the protocol. I tried to follow the xen handshaking protocol between front and back end at startup. Yes, that's the right direction. Following existing convention is good enough for an intern project. Specifying the protocol in detailed is not the requirement for a prototype. But in the end to upstream xen-9pfs a canonical protocol is required. A blessed version of protocol needs to be committed to xen.git tree. We have a bunch of those in xen.git/xen/include/public/io/ directory. Wei. Thank you. L Thanks. Linda As for "9pvirtio specific code", I think there is misunderstanding because though a lot of files in QEMU are prefixed with virtio they are actually not specific to virtio at all. I think the "independent of virtio ..." part was referring to the new transport she wrote. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 9p file system for xen
On 12/1/2015 4:47 AM, Wei Liu wrote: On Mon, Nov 30, 2015 at 12:19:18PM -0500, Neil Sikka wrote: Hi Wei, could you please explain why/how you would have done the project differently now and why these patches are not "good"? From my conversation with Linda, I understood that her code is "Independent of virtio except the 9pvirtio specific code, which is used extensively." I need to implement a xen transport for 9pfs. Linda was essentially doing the same. But she didn't specify the canonical protocol between frontend and backend. For my own edification: In the interests of the limited time of my internship, we decided I shouldn't do the initialization using the xen toolstack. Were there are other expediencies that I'm unaware of? I tried to follow the xen handshaking protocol between front and back end at startup. Thanks. Linda As for "9pvirtio specific code", I think there is misunderstanding because though a lot of files in QEMU are prefixed with virtio they are actually not specific to virtio at all. I think the "independent of virtio ..." part was referring to the new transport she wrote. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 9p file system for xen
Hi Wei, On 11/19/2015 8:03 AM, Wei Liu wrote: On Thu, Nov 19, 2015 at 09:55:00AM -0500, Neil Sikka wrote: Is there any documentation about planned interfaces and API contracts for people building around the virtio/9pfs layers? For example, while this is I assume that you're interested in getting 9pfs to work but don't care much about how it is made to work? I ask because I'm a bit confused by the notion of "virtio/9pfs" because what Linda did wasn't based on virtio transport. A clarification: If you mean the virtio 9pfs transport, my code is based on that, and sits on top of it. still getting debugged/checked in, in order to build DomU support for these devices, the expected API contracts/interfaces would need to be known. I'm not sure I get your question. What do you mean by "build DomU support"? What kind of DomU support needs insider knowledge of how 9pfs is setup? Are you talking about your own management stack that starts VM? Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 9p file system for xen
On 11/17/2015 11:35 AM, Neil Sikka wrote: How does Linda's work relate to Wei's patches available here (I didnt see them in Xen-4.6.0): http://downloads.xen.org/Wiki/VirtioOnXen/qemu-01-xenpv-exec.patch http://downloads.xen.org/Wiki/VirtioOnXen/qemu-02-virtio-for-pv.patch I'll let Wei answer this. Also, since 9p is being worked on, which is a filesystem that should be implemented in a kernel rather than a hypervisor, are you looking to contribute this driver to the Linux kernel? What I did was write new kernel routines and new Qemu routines, as well as modifying a few existing Qemu files. The initialization is currently done manually by modifying xenstore. This is the only code that properly belongs in the hypervisor. I hope this clarifies things. Linda On Mon, Nov 16, 2015 at 10:02 PM, Linda <mailto:lin...@jma3.com>> wrote: Hi Wei, On 11/16/2015 10:35 AM, Wei Liu wrote: On Mon, Nov 16, 2015 at 10:22:41AM -0700, Linda wrote: ... The bug is a timing issue: During virtio's probe step, on the front end, it initialized the mount path. Since at that time, the front end doesn't have access to the back end's entries in xenstore (AFIACT), I either need to put it in xenstore prior to starting, or move the access to this information to later in the initialization. Note, I used the past tense on what virtio did, as of last summer: when I looked at it last week, it appears to have changed since I first used it as a template.I need to investigate this further. OK. Finally, I've made no provision for how to mount more than one file system for the same guest. This is a feature that virtio provides for in the front-end code (as do I), but I am unclear about how this works in the back-end or at the user level. This is what I suspect will be different in xen, and I'd like some input on what it should look like. I think this comes down to how your design the xenstore protocol to represent different mount points. And just reading this gave me the answer I need. The code freeze for next release is going to be end of March next year. As software engineer often overestimates the progress he or she can make, I would say we shall aim for getting something working as soon as possible. Get the design straight and something clean by the end of this year would be good. Sounds good to me. I'm happy to keep working on this. I just didn't want to find myself in a position where I needed to pass this on to someone else, but I didn't give that person enough time to finish what I'd done. Depending on the situation, I can take over the code. You've done enough for this project and we don't really want you to work on it for free -- we don't have provision for more funding at the moment. Understood. If we end up taking over the project, we will still attribute the initial implementation to you. Thanks. Julien said essentially the same thing. Right now, I'm working on average, less than 10 hours/week, so it's enough to keep my mind engaged, but it doesn't interfere with anything else. I will be working for pay, in some capacity (TBD), after the first of the year. Right now, I'm working to line things up. Unless something changes drastically, I'll continue to work on this until the end of the year. I'll start by cleaning things up, and keep it that way, so no matter what happens, you, or Julien, can take it over. Linda Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org <mailto:Xen-devel@lists.xen.org> http://lists.xen.org/xen-devel -- My Blog: http://www.neilscomputerblog.blogspot.com/ Twitter: @neilsikka ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 9p file system for xen
Hi Wei, On 11/16/2015 10:35 AM, Wei Liu wrote: On Mon, Nov 16, 2015 at 10:22:41AM -0700, Linda wrote: ... The bug is a timing issue: During virtio's probe step, on the front end, it initialized the mount path. Since at that time, the front end doesn't have access to the back end's entries in xenstore (AFIACT), I either need to put it in xenstore prior to starting, or move the access to this information to later in the initialization. Note, I used the past tense on what virtio did, as of last summer: when I looked at it last week, it appears to have changed since I first used it as a template.I need to investigate this further. OK. Finally, I've made no provision for how to mount more than one file system for the same guest. This is a feature that virtio provides for in the front-end code (as do I), but I am unclear about how this works in the back-end or at the user level. This is what I suspect will be different in xen, and I'd like some input on what it should look like. I think this comes down to how your design the xenstore protocol to represent different mount points. And just reading this gave me the answer I need. The code freeze for next release is going to be end of March next year. As software engineer often overestimates the progress he or she can make, I would say we shall aim for getting something working as soon as possible. Get the design straight and something clean by the end of this year would be good. Sounds good to me. I'm happy to keep working on this. I just didn't want to find myself in a position where I needed to pass this on to someone else, but I didn't give that person enough time to finish what I'd done. Depending on the situation, I can take over the code. You've done enough for this project and we don't really want you to work on it for free -- we don't have provision for more funding at the moment. Understood. If we end up taking over the project, we will still attribute the initial implementation to you. Thanks. Julien said essentially the same thing. Right now, I'm working on average, less than 10 hours/week, so it's enough to keep my mind engaged, but it doesn't interfere with anything else. I will be working for pay, in some capacity (TBD), after the first of the year. Right now, I'm working to line things up. Unless something changes drastically, I'll continue to work on this until the end of the year. I'll start by cleaning things up, and keep it that way, so no matter what happens, you, or Julien, can take it over. Linda Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 9p file system for xen
On 11/16/2015 9:51 AM, Wei Liu wrote: On Mon, Nov 16, 2015 at 09:36:24AM -0700, Linda wrote: Hi Wei, On 11/16/2015 8:16 AM, Wei Liu wrote: Hi Linda On Fri, Nov 13, 2015 at 10:23:22AM -0700, Linda wrote: Hello, I worked this summer as an intern under Julien Grall and Wei Liu. My project was to develop a prototype/proof of concept xen front/back end for the 9p file system. I mostly hacked the virtio 9p system. This project was not complete, at the end of the summer. Julien said that you all wanted to include this in the next release of xen in January, and offered to take it over. I told Julien I wanted to continue working on it, which I have been doing, very much in the background. I came upon a bug in my code recently that made me aware that I am not clear what the expectation for what I deliver should be: i.e., whether it's still a prototype, or whether this should be production software. Right now, I do not modify the toolstack (I never learned how), but rather start and pause my guest, and then modify xenstore, manually. I can fix my bug in the same manner, but this will limit the usefulness of what I deliver. To do more will hit up against the limitations of my time and knowledge. So please let me know what you're expecting, especially wrt the user interface, and when I would need to complete everything for this release. If I interpret this correctly, you have a prototype that's working? Do you have your code somewhere? No. I hit a bug that I would fix differently, depending on my goal. I think we would still like to include it in next release if possible -- that would require a properly implemented solution, not just a prototype. Let's assess the current situation and then decide what to The situation is, given my current knowledge and what my availability has been (it may improve), I can either: a. Get a decent prototype working by the end of the year. This would have certain values pre-written in xenstore, that I'm currently doing manually. There are potentially some issues with mounting that I suspect need to be different for xen than they are for virtio - so either way, I need a clarification of how xen people want this to work. b. Make sure what I've written is working, and pass it on to someone else to update the toolstack, and resolve the issues, described above. In this scenario, I would need to know how much time that someone would need and just devote a week to getting this to them. Your description is too vague. I don't have clear idea what kind of bug you encountered and what suggestion I can give. The bug is a timing issue: During virtio's probe step, on the front end, it initialized the mount path. Since at that time, the front end doesn't have access to the back end's entries in xenstore (AFIACT), I either need to put it in xenstore prior to starting, or move the access to this information to later in the initialization. Note, I used the past tense on what virtio did, as of last summer: when I looked at it last week, it appears to have changed since I first used it as a template.I need to investigate this further. Finally, I've made no provision for how to mount more than one file system for the same guest. This is a feature that virtio provides for in the front-end code (as do I), but I am unclear about how this works in the back-end or at the user level. This is what I suspect will be different in xen, and I'd like some input on what it should look like. The code freeze for next release is going to be end of March next year. As software engineer often overestimates the progress he or she can make, I would say we shall aim for getting something working as soon as possible. Get the design straight and something clean by the end of this year would be good. Sounds good to me. I'm happy to keep working on this. I just didn't want to find myself in a position where I needed to pass this on to someone else, but I didn't give that person enough time to finish what I'd done. Either way, my next step is to sync up my qemu with the current qemu, and merge everything, and then my github will be correct, at which point you'll be able to access my most recent code. That would be a good first step. You don't actually need to fix the bug for that if you don't know how to proceed yet. Good. I'm glad we're on the same page, on this. Linda Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 9p file system for xen
Hi Wei, On 11/16/2015 8:16 AM, Wei Liu wrote: Hi Linda On Fri, Nov 13, 2015 at 10:23:22AM -0700, Linda wrote: Hello, I worked this summer as an intern under Julien Grall and Wei Liu. My project was to develop a prototype/proof of concept xen front/back end for the 9p file system. I mostly hacked the virtio 9p system. This project was not complete, at the end of the summer. Julien said that you all wanted to include this in the next release of xen in January, and offered to take it over. I told Julien I wanted to continue working on it, which I have been doing, very much in the background. I came upon a bug in my code recently that made me aware that I am not clear what the expectation for what I deliver should be: i.e., whether it's still a prototype, or whether this should be production software. Right now, I do not modify the toolstack (I never learned how), but rather start and pause my guest, and then modify xenstore, manually. I can fix my bug in the same manner, but this will limit the usefulness of what I deliver. To do more will hit up against the limitations of my time and knowledge. So please let me know what you're expecting, especially wrt the user interface, and when I would need to complete everything for this release. If I interpret this correctly, you have a prototype that's working? Do you have your code somewhere? No. I hit a bug that I would fix differently, depending on my goal. I think we would still like to include it in next release if possible -- that would require a properly implemented solution, not just a prototype. Let's assess the current situation and then decide what to The situation is, given my current knowledge and what my availability has been (it may improve), I can either: a. Get a decent prototype working by the end of the year. This would have certain values pre-written in xenstore, that I'm currently doing manually. There are potentially some issues with mounting that I suspect need to be different for xen than they are for virtio - so either way, I need a clarification of how xen people want this to work. b. Make sure what I've written is working, and pass it on to someone else to update the toolstack, and resolve the issues, described above. In this scenario, I would need to know how much time that someone would need and just devote a week to getting this to them. Either way, my next step is to sync up my qemu with the current qemu, and merge everything, and then my github will be correct, at which point you'll be able to access my most recent code. Linda do next. Wei. Thank you. Linda Jacobson ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] 9p file system for xen
Hello, I worked this summer as an intern under Julien Grall and Wei Liu. My project was to develop a prototype/proof of concept xen front/back end for the 9p file system. I mostly hacked the virtio 9p system. This project was not complete, at the end of the summer. Julien said that you all wanted to include this in the next release of xen in January, and offered to take it over. I told Julien I wanted to continue working on it, which I have been doing, very much in the background. I came upon a bug in my code recently that made me aware that I am not clear what the expectation for what I deliver should be: i.e., whether it's still a prototype, or whether this should be production software. Right now, I do not modify the toolstack (I never learned how), but rather start and pause my guest, and then modify xenstore, manually. I can fix my bug in the same manner, but this will limit the usefulness of what I deliver. To do more will hit up against the limitations of my time and knowledge. So please let me know what you're expecting, especially wrt the user interface, and when I would need to complete everything for this release. Thank you. Linda Jacobson ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] grant tables and driver handshaking
On 6/11/2015 10:38 AM, Ian Campbell wrote: On Thu, 2015-06-11 at 17:24 +0100, Wei Liu wrote: Since the backend is in DOM0, and is around longer than the DOMUs, this happens first. It's that interaction that I'd like a little more text on. If it exists. I'm not completely sure what you're referring to. That probably only exists in code. The interaction between backend and frontend is very driver specific. Most front and backend pairs follow a similar pattern though, and it is best not to reinvent the wheel completely, even if some of the details differ. In general everything is driven from a pair of "state" nodes in xenstore, one in the frontend dir and one in the backend dir. These contain the enum xenbus_state from xen/include/public/io/xenbus.h (as %d formatted integers). In general both front and backend start in state 1 (XenbusStateInitialising) and each has a watch on the other end. Things generally start with the backend moving to state 2 (XenbusStateInitWait) and from there they progress n lockstep through the stages until they both reach state 4 (XenbusStateConnected), then you are up and running. On teardown one end goes to 5 (XenbusStateClosing) and the other follows. Thanks, Ian. This helped a lot. And thanks for the links, below. Linda In Linux the place to look is the various "otherend_changed" hooks in the front and back drivers, usually the backend half is in a xenbus.c file (e.g. netback and blkback). blkif.h has a description of the states in that case. http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,io,blkif.h.html I'm not aware of any particular docs on the general case, or at least I can't find the ones I thought we had. Ian ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] grant tables and driver handshaking
On 6/11/2015 4:43 AM, Wei Liu wrote: On Wed, Jun 10, 2015 at 07:45:15PM -0600, Linda wrote: Hello all, I will be writing a xen front and back-end pair for a 9p transport. I have two areas where I'm still a little more muddled than I'd like to be. Can anyone please recommend a good article on either grant tables or the handshaking between the front and back-end drivers, or both? The handshake protocol needs to be designed by you. Generally speaking you need to at least advertise the event channel and grant table references. You also need to advertise other information that you identify as part of the handshake process. OK. That explains half of my muddle. Here's the other half. The wiki article on xenbus taks about the front-end registering with xenbus, before anything else happens. But in blkback, it looks like it too has to do some kind of setup with xenbus, too. Since the backend is in DOM0, and is around longer than the DOMUs, this happens first. It's that interaction that I'd like a little more text on. If it exists. There is a wiki page on grant table http://wiki.xenproject.org/wiki/Grant_Table Thanks. Wei. I have looked at the code, but could use more verbiage. Thanks. Linda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] grant tables and driver handshaking
Hello all, I will be writing a xen front and back-end pair for a 9p transport. I have two areas where I'm still a little more muddled than I'd like to be. Can anyone please recommend a good article on either grant tables or the handshaking between the front and back-end drivers, or both? I have looked at the code, but could use more verbiage. Thanks. Linda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] clarification of xen Wiki article
On 6/4/2015 3:58 AM, Ian Campbell wrote: On Wed, 2015-06-03 at 09:24 -0600, Linda wrote: Hi all, I was reading http://wiki.xen.org/wiki/XenBus In the section on Store Organization, there is the statement "Information should not be replicated in the store and required to be consistent " I'm not sure what that means. Can anybody clarify this for me, please? This came from the old wiki, so the history is unclear. I suppose it could mean either: If you replicate information in xenstore then you should not expect it to be consistent or Do not replicate information, do not require information to be consistent. But let me turn the question around: What are you actually trying to achieve? i.e. what is causing this ambiguous and confusing statement to be a concern for you. Sometime soon I will be writing a front- and back- end for the 9p fs protocol. (My Outreachy project). My goall is to understand where drivers sit logically and physically, so I know what I'm doing. Part of this is to understand Xenbus and Xenstore. When I know too little, I read carefully, and flag anything I don't understand. For now, I'll ignore this statement. Thanks, Ian. Linda Jacobson Unless the author or someone else in the know speaks up I would propose to remove this text from the wiki. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] clarification of xen Wiki article
Hi all, I was reading http://wiki.xen.org/wiki/XenBus In the section on Store Organization, there is the statement "Information should not be replicated in the store and required to be consistent " I'm not sure what that means. Can anybody clarify this for me, please? Thanks. linda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v7] libxl: provide libxl_bitmap_{or,and}
On 4/16/2015 8:45 AM, Wei Liu wrote: On Thu, Apr 16, 2015 at 08:40:52AM -0600, Linda wrote: On 4/16/2015 8:34 AM, Ian Campbell wrote: On Wed, 2015-04-15 at 11:02 -0600, Linda Jacobson wrote: New functions to provide logical and and or of two bitmaps functions can be used in vNUMA configuration check function. I don't think that really justifies adding them, since we aren't actually using them for that now, how about: New functions to provide logical and and or of two bitmaps. These are generically useful utility functions added to the public API for the benefit of libxl's users. In the future they may also be useful internally, e.g. in the vNUMA configuration check function. If you are happy with that, and there's no other reason to resend, then I can switch in that text as I commit. I'm fine with this. If Wei and Julien are, please go ahead. Julien is away now. I'm of course fine with this -- it's your contribution after all, you have the final say. :-) Then, Ian, please, go ahead. And congratulations to you for getting first patch accepted! Why, thank you. Linda Wei. Signed-off-by: Linda Jacobson --- v.1 The new functions were added. v.2 The comments and format were corrected. v.3 The bitmap functions were rewritten to manipulate bytes not bits. v.4 Several non-modified parameters, and local variables were changed to const Also the code formatting was fixed. v.5 The commit subject line now has versions and is simpler. v.6 All descriptions in the commit history are now complete sentences. Extraneous blank lines are gone. v.7 Added LIBXL_BIT_HAVE_AND_OR to libxl.h; deleted extraneous comment in libxl_utils.h; and updated the commit log to include the uses for these functions --- tools/libxl/libxl.h | 9 ++ tools/libxl/libxl_utils.c | 70 +++ tools/libxl/libxl_utils.h | 6 3 files changed, 85 insertions(+) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 5eec092..1fb9549 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -84,6 +84,15 @@ #define LIBXL_HAVE_CPUPOOL_QUALIFIER_TO_CPUPOOLID 1 /* + * + * LIBXL_HAVE_BITMAP_AND_OR + * + * If this is defined, libxl has two libarary functions, libxl_bitmap_and "library". (again, can fix on commit) Thanks for catching the typo. Linda Jacobson Other than those: Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v7] libxl: provide libxl_bitmap_{or,and}
On 4/16/2015 8:34 AM, Ian Campbell wrote: On Wed, 2015-04-15 at 11:02 -0600, Linda Jacobson wrote: New functions to provide logical and and or of two bitmaps functions can be used in vNUMA configuration check function. I don't think that really justifies adding them, since we aren't actually using them for that now, how about: New functions to provide logical and and or of two bitmaps. These are generically useful utility functions added to the public API for the benefit of libxl's users. In the future they may also be useful internally, e.g. in the vNUMA configuration check function. If you are happy with that, and there's no other reason to resend, then I can switch in that text as I commit. I'm fine with this. If Wei and Julien are, please go ahead. Signed-off-by: Linda Jacobson --- v.1 The new functions were added. v.2 The comments and format were corrected. v.3 The bitmap functions were rewritten to manipulate bytes not bits. v.4 Several non-modified parameters, and local variables were changed to const Also the code formatting was fixed. v.5 The commit subject line now has versions and is simpler. v.6 All descriptions in the commit history are now complete sentences. Extraneous blank lines are gone. v.7 Added LIBXL_BIT_HAVE_AND_OR to libxl.h; deleted extraneous comment in libxl_utils.h; and updated the commit log to include the uses for these functions --- tools/libxl/libxl.h | 9 ++ tools/libxl/libxl_utils.c | 70 +++ tools/libxl/libxl_utils.h | 6 3 files changed, 85 insertions(+) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 5eec092..1fb9549 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -84,6 +84,15 @@ #define LIBXL_HAVE_CPUPOOL_QUALIFIER_TO_CPUPOOLID 1 /* + * + * LIBXL_HAVE_BITMAP_AND_OR + * + * If this is defined, libxl has two libarary functions, libxl_bitmap_and "library". (again, can fix on commit) Thanks for catching the typo. Linda Jacobson Other than those: Acked-by: Ian Campbell ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7] libxl: provide libxl_bitmap_{or,and}
New functions to provide logical and and or of two bitmaps functions can be
used in vNUMA configuration check function. They are also generally useful, so
they have been made as public API.
Signed-off-by: Linda Jacobson
---
v.1 The new functions were added.
v.2 The comments and format were corrected.
v.3 The bitmap functions were rewritten to manipulate bytes not bits.
v.4 Several non-modified parameters, and local variables were changed to const
Also the code formatting was fixed.
v.5 The commit subject line now has versions and is simpler.
v.6 All descriptions in the commit history are now complete sentences.
Extraneous blank lines are gone.
v.7 Added LIBXL_BIT_HAVE_AND_OR to libxl.h; deleted extraneous comment in
libxl_utils.h; and updated the commit log to include the uses for these
functions
---
tools/libxl/libxl.h | 9 ++
tools/libxl/libxl_utils.c | 70 +++
tools/libxl/libxl_utils.h | 6
3 files changed, 85 insertions(+)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5eec092..1fb9549 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -84,6 +84,15 @@
#define LIBXL_HAVE_CPUPOOL_QUALIFIER_TO_CPUPOOLID 1
/*
+ *
+ * LIBXL_HAVE_BITMAP_AND_OR
+ *
+ * If this is defined, libxl has two libarary functions, libxl_bitmap_and
+ * and libxl_bitmap_or to compute the logical and and or of two bitmaps
+ */
+#define LIBXL_HAVE_BITMAP_AND_OR 1
+
+/*
* LIBXL_HAVE_FIRMWARE_PASSTHROUGH indicates the feature for
* passing in SMBIOS and ACPI firmware to HVM guests is present
* in the library.
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..f552700 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,76 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, or_map, large_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't the same size, their union (logical or) will
+ * be size of larger bit map. Any bit past the end of the
+ * smaller bit map, will match the larger one.
+ */
+for (i = 0; i < small_map->size; i++)
+or_map->map[i] = (small_map->map[i] | large_map->map[i]);
+
+for (i = small_map->size; i < large_map->size; i++)
+or_map->map[i] = large_map->map[i];
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, and_map, small_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't same size, their 'and' will be size of
+ * smaller bit map
+ */
+for (i = 0; i < and_map->size; i++)
+and_map->map[i] = (large_map->map[i] & small_map->map[i]);
+
+out:
+GC_FREE;
+return rc;
+}
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..f5a3168 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,12 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1,
+const libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1,
+ const libxl_bitmap *map2);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}
On 4/15/2015 10:43 AM, Wei Liu wrote: On Wed, Apr 15, 2015 at 10:38:19AM -0600, Linda wrote: BTW who changes the configure file to test for the HAVE_ macros? The user (application) of libxl should test that. We don't need to worry about the test. Thanks. Linda Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}
BTW who changes the configure file to test for the HAVE_ macros?
Thanks.
Linda
On 4/15/2015 7:41 AM, Wei Liu wrote:
On Wed, Apr 15, 2015 at 02:29:10PM +0100, Ian Campbell wrote:
On Wed, 2015-04-15 at 14:15 +0100, Wei Liu wrote:
On Wed, Apr 15, 2015 at 01:45:14PM +0100, Ian Campbell wrote:
On Wed, 2015-04-15 at 05:45 -0600, Linda Jacobson wrote:
There are new functions to provide logical and and or of two bitmaps.
Please could you add a sentence or two on the intended use of these
functions, since there are no callers being added here.
Linda is our Outreachy applicant. This is a small task that Julien and I
assigned to her.
Sure, but that doesn't remove the need for the commit log to be a
standalone justification for the patch in its own right.
One user I can think of is in some of the vNUMA validation functions
that operate on bitmaps. But to keep this task small and simple I didn't
ask her to actually use the functions she introduce.
In particular without that I can't tell if these need to be part of the
public API or if they are going to be used by something internal.
I think these functions should be public functions.
Sure, but the reasoning for why you^WLinda thinks that needs to be in
the commit log. In particular because there are no users being added.
I could probably guess why you think these should be public, but I
shouldn't have to guess and in any case that doesn't help in 6 months
when someone asks "why do we have these functions".
Linda, can you use something like this in your commit log:
Provide libxl_bitmap_{and,or} functions. These functions can be used in
vNUMA configuration check function. They are also generally useful so
they are made as public API.
If the former then I think a LIBXL_HAVE_... #define is needed in libxl.h
(as described by the comments in there, and there are many examples).
We're a bit lax on these functions (there is no LIBXL_HAVE_BITMAP...)
Weren't they always there and hence don't need it?
OK, maybe I'm mistaken.
Linda, you need to add one more hunk to libxl.h.
#define LIBXL_HAVE_BITMAP_AND_OR 1
Search for other LIBXL_HAVE_ macro to get an idea how it is handled.
If you have any questions, just ask.
Wei.
Ian.
so
I didn't ask her to add one. We can add LIBXL_HAVE_BITMAP_AND_OR in
libxl.h if you think it is necessary.
Wei.
If the latter then the prototype should differ. (I'll explain that
if/when this turns out to be the case).
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..a128a7c 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,13 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+/* Or and and functions for two bitmaps */
This comment doesn't say anything which isn't already apparent from the
names and prototypes of the functions in question. Just leave it out.
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1,
+const libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1,
+ const libxl_bitmap *map2);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}
On 4/15/2015 9:54 AM, Ian Campbell wrote: On Wed, 2015-04-15 at 16:50 +0100, Wei Liu wrote: On Wed, Apr 15, 2015 at 09:13:51AM -0600, Linda wrote: When adding the HAVE macro, is there a protocol on where in libxl.h, this one should be placed? They appear to be fairly spread out through the file. You can place it near the top. But not right at the top! Just stick it next to an arbitrary existing one. Will do. Linda Jacobson ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}
When adding the HAVE macro, is there a protocol on where in libxl.h,
this one should be placed? They appear to be fairly spread out through
the file.
Thanks.
Linda
On 4/15/2015 7:41 AM, Wei Liu wrote:
On Wed, Apr 15, 2015 at 02:29:10PM +0100, Ian Campbell wrote:
On Wed, 2015-04-15 at 14:15 +0100, Wei Liu wrote:
On Wed, Apr 15, 2015 at 01:45:14PM +0100, Ian Campbell wrote:
On Wed, 2015-04-15 at 05:45 -0600, Linda Jacobson wrote:
There are new functions to provide logical and and or of two bitmaps.
Please could you add a sentence or two on the intended use of these
functions, since there are no callers being added here.
Linda is our Outreachy applicant. This is a small task that Julien and I
assigned to her.
Sure, but that doesn't remove the need for the commit log to be a
standalone justification for the patch in its own right.
One user I can think of is in some of the vNUMA validation functions
that operate on bitmaps. But to keep this task small and simple I didn't
ask her to actually use the functions she introduce.
In particular without that I can't tell if these need to be part of the
public API or if they are going to be used by something internal.
I think these functions should be public functions.
Sure, but the reasoning for why you^WLinda thinks that needs to be in
the commit log. In particular because there are no users being added.
I could probably guess why you think these should be public, but I
shouldn't have to guess and in any case that doesn't help in 6 months
when someone asks "why do we have these functions".
Linda, can you use something like this in your commit log:
Provide libxl_bitmap_{and,or} functions. These functions can be used in
vNUMA configuration check function. They are also generally useful so
they are made as public API.
If the former then I think a LIBXL_HAVE_... #define is needed in libxl.h
(as described by the comments in there, and there are many examples).
We're a bit lax on these functions (there is no LIBXL_HAVE_BITMAP...)
Weren't they always there and hence don't need it?
OK, maybe I'm mistaken.
Linda, you need to add one more hunk to libxl.h.
#define LIBXL_HAVE_BITMAP_AND_OR 1
Search for other LIBXL_HAVE_ macro to get an idea how it is handled.
If you have any questions, just ask.
Wei.
Ian.
so
I didn't ask her to add one. We can add LIBXL_HAVE_BITMAP_AND_OR in
libxl.h if you think it is necessary.
Wei.
If the latter then the prototype should differ. (I'll explain that
if/when this turns out to be the case).
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..a128a7c 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,13 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+/* Or and and functions for two bitmaps */
This comment doesn't say anything which isn't already apparent from the
names and prototypes of the functions in question. Just leave it out.
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1,
+const libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1,
+ const libxl_bitmap *map2);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}
Sent from my iPhone
> On Apr 15, 2015, at 7:41 AM, Wei Liu wrote:
>
>> On Wed, Apr 15, 2015 at 02:29:10PM +0100, Ian Campbell wrote:
>>> On Wed, 2015-04-15 at 14:15 +0100, Wei Liu wrote:
>>>> On Wed, Apr 15, 2015 at 01:45:14PM +0100, Ian Campbell wrote:
>>>>> On Wed, 2015-04-15 at 05:45 -0600, Linda Jacobson wrote:
>>>>> There are new functions to provide logical and and or of two bitmaps.
>>>>
>>>> Please could you add a sentence or two on the intended use of these
>>>> functions, since there are no callers being added here.
>>>
>>> Linda is our Outreachy applicant. This is a small task that Julien and I
>>> assigned to her.
>>
>> Sure, but that doesn't remove the need for the commit log to be a
>> standalone justification for the patch in its own right.
>>
>>> One user I can think of is in some of the vNUMA validation functions
>>> that operate on bitmaps. But to keep this task small and simple I didn't
>>> ask her to actually use the functions she introduce.
>>>
>>>> In particular without that I can't tell if these need to be part of the
>>>> public API or if they are going to be used by something internal.
>>>
>>> I think these functions should be public functions.
>>
>> Sure, but the reasoning for why you^WLinda thinks that needs to be in
>> the commit log. In particular because there are no users being added.
>>
>> I could probably guess why you think these should be public, but I
>> shouldn't have to guess and in any case that doesn't help in 6 months
>> when someone asks "why do we have these functions".
>
> Linda, can you use something like this in your commit log:
>
> Provide libxl_bitmap_{and,or} functions. These functions can be used in
> vNUMA configuration check function. They are also generally useful so
> they are made as public API.
Of course
L
>
>>>> If the former then I think a LIBXL_HAVE_... #define is needed in libxl.h
>>>> (as described by the comments in there, and there are many examples).
>>>
>>> We're a bit lax on these functions (there is no LIBXL_HAVE_BITMAP...)
>>
>> Weren't they always there and hence don't need it?
>
> OK, maybe I'm mistaken.
>
> Linda, you need to add one more hunk to libxl.h.
>
> #define LIBXL_HAVE_BITMAP_AND_OR 1
>
> Search for other LIBXL_HAVE_ macro to get an idea how it is handled.
>
> If you have any questions, just ask.
>
> Wei.
>
>> Ian.
>>
>>> so
>>> I didn't ask her to add one. We can add LIBXL_HAVE_BITMAP_AND_OR in
>>> libxl.h if you think it is necessary.
>>>
>>> Wei.
>>>
>>>> If the latter then the prototype should differ. (I'll explain that
>>>> if/when this turns out to be the case).
>>>>
>>>>> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
>>>>> index acacdd9..a128a7c 100644
>>>>> --- a/tools/libxl/libxl_utils.h
>>>>> +++ b/tools/libxl/libxl_utils.h
>>>>> @@ -90,6 +90,13 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int
>>>>> bit);
>>>>> void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
>>>>> void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
>>>>> int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
>>>>> +/* Or and and functions for two bitmaps */
>>>>
>>>> This comment doesn't say anything which isn't already apparent from the
>>>> names and prototypes of the functions in question. Just leave it out.
>>>>
>>>>
>>>>> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
>>>>> +const libxl_bitmap *map1,
>>>>> +const libxl_bitmap *map2);
>>>>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>>>>> + const libxl_bitmap *map1,
>>>>> + const libxl_bitmap *map2);
>>>>> char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap
>>>>> *cpumap);
>>>>> static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
>>>>> {
>>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}
There are new functions to provide logical and and or of two bitmaps.
Signed-off-by: Linda Jacobson
---
v.1 The new functions were added.
v.2 The comments and format were corrected.
v.3 The bitmap functions were rewritten to manipulate bytes not bits.
v.4 Several non-modified parameters, and local variables were changed to const
Also the code formatting was fixed.
v.5 The commit subject line now has versions and is simpler.
v.6 All descriptions in the commit history are now complete sentences.
Extraneous blank lines are gone.
---
tools/libxl/libxl_utils.c | 70 +++
tools/libxl/libxl_utils.h | 7 +
2 files changed, 77 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..f552700 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,76 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, or_map, large_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't the same size, their union (logical or) will
+ * be size of larger bit map. Any bit past the end of the
+ * smaller bit map, will match the larger one.
+ */
+for (i = 0; i < small_map->size; i++)
+or_map->map[i] = (small_map->map[i] | large_map->map[i]);
+
+for (i = small_map->size; i < large_map->size; i++)
+or_map->map[i] = large_map->map[i];
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, and_map, small_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't same size, their 'and' will be size of
+ * smaller bit map
+ */
+for (i = 0; i < and_map->size; i++)
+and_map->map[i] = (large_map->map[i] & small_map->map[i]);
+
+out:
+GC_FREE;
+return rc;
+}
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..a128a7c 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,13 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+/* Or and and functions for two bitmaps */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1,
+const libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1,
+ const libxl_bitmap *map2);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [[PATCH v4]] new functions libxl_bitmap_{or,and}
Not ignored. Sorry. I shouldn't have read from my iPhone. I didn't
see the others. I'll take care of them.
Linda
On 4/14/2015 10:33 AM, Wei Liu wrote:
Urgh... I think I made a mistake in the rune I gave you, sorry. The
--subject-prefix= doesn't need to include "[]".
And you forgot to change the subject line to
libxl: provide libxl_bitmap_{and,or}
I'm a picky about the subject line because this is what shows up when
you look at git commit log.
On Tue, Apr 14, 2015 at 08:07:59AM -0600, Linda Jacobson wrote:
provide logical and and or of two bitmaps
Provide logical and and or of two bitmaps.
This should be a proper sentence.
Other than these minor nits the code logic looks OK.
Signed-off-by: Linda Jacobson
---
[...]
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+
We only need one blank line here.
+rc = libxl_bitmap_alloc(ctx, and_map, small_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't same size, their 'and' will be size of
+ * smaller bit map
+ */
+for (i = 0; i < and_map->size; i++)
+and_map->map[i] = (large_map->map[i] & small_map->map[i]);
+
+out:
+GC_FREE;
+return rc;
+
No need to have blank lines after "return rc;"
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5] libxl: provide libxl_bitmap_{or,and}
provide logical and and or of two bitmaps
Signed-off-by: Linda Jacobson
---
v.1 new functions
v.2 updated comments and format
v.3 rewrote bitmap functions to manipulate bytes not bits
v.4 made non-modified parameters, and local variables const; fixed code
formatting
v.5 changed commit subject line
---
tools/libxl/libxl_utils.c | 73 +++
tools/libxl/libxl_utils.h | 7 +
2 files changed, 80 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..2241da9 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,79 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, or_map, large_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't the same size, their union (logical or) will
+ * be size of larger bit map. Any bit past the end of the
+ * smaller bit map, will match the larger one.
+ */
+for (i = 0; i < small_map->size; i++)
+or_map->map[i] = (small_map->map[i] | large_map->map[i]);
+
+for (i = small_map->size; i < large_map->size; i++)
+or_map->map[i] = large_map->map[i];
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+
+rc = libxl_bitmap_alloc(ctx, and_map, small_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't same size, their 'and' will be size of
+ * smaller bit map
+ */
+for (i = 0; i < and_map->size; i++)
+and_map->map[i] = (large_map->map[i] & small_map->map[i]);
+
+out:
+GC_FREE;
+return rc;
+
+}
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..a128a7c 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,13 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+/* Or and and functions for two bitmaps */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1,
+const libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1,
+ const libxl_bitmap *map2);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [[PATCH v4]] new functions libxl_bitmap_{or,and}
I'll fix it when I get home. It'll be late where you are. L
Sent from my iPhone
> On Apr 14, 2015, at 10:33 AM, Wei Liu wrote:
>
> Urgh... I think I made a mistake in the rune I gave you, sorry. The
> --subject-prefix= doesn't need to include "[]".
>
> And you forgot to change the subject line to
> libxl: provide libxl_bitmap_{and,or}
>
> I'm a picky about the subject line because this is what shows up when
> you look at git commit log.
>
>> On Tue, Apr 14, 2015 at 08:07:59AM -0600, Linda Jacobson wrote:
>> provide logical and and or of two bitmaps
>
> Provide logical and and or of two bitmaps.
>
> This should be a proper sentence.
>
> Other than these minor nits the code logic looks OK.
>
>>
>> Signed-off-by: Linda Jacobson
>>
>> ---
> [...]
>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>> + const libxl_bitmap *map1, const libxl_bitmap *map2)
>> +{
>> +GC_INIT(ctx);
>> +int rc;
>> +uint32_t i;
>> +const libxl_bitmap *large_map;
>> +const libxl_bitmap *small_map;
>> +
>> +if (map1->size > map2->size) {
>> +large_map = map1;
>> +small_map = map2;
>> +} else {
>> +large_map = map2;
>> +small_map = map1;
>> +}
>> +
>> +
>
> We only need one blank line here.
>
>> +rc = libxl_bitmap_alloc(ctx, and_map, small_map->size * 8);
>> +if (rc)
>> +goto out;
>> +
>> +/*
>> + * If bitmaps aren't same size, their 'and' will be size of
>> + * smaller bit map
>> + */
>> +for (i = 0; i < and_map->size; i++)
>> +and_map->map[i] = (large_map->map[i] & small_map->map[i]);
>> +
>> +out:
>> +GC_FREE;
>> +return rc;
>> +
>
> No need to have blank lines after "return rc;"
>
> Wei.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [[PATCH v4]] new functions libxl_bitmap_{or,and}
provide logical and and or of two bitmaps
Signed-off-by: Linda Jacobson
---
v.1 new functions
v.2 updated comments and format
v.3 rewrote bitmap functions to manipulate bytes not bits
v.4 made non-modified parameters, and local variables const; fixed code
formatting
---
tools/libxl/libxl_utils.c | 73 +++
tools/libxl/libxl_utils.h | 7 +
2 files changed, 80 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..2241da9 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,79 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, or_map, large_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't the same size, their union (logical or) will
+ * be size of larger bit map. Any bit past the end of the
+ * smaller bit map, will match the larger one.
+ */
+for (i = 0; i < small_map->size; i++)
+or_map->map[i] = (small_map->map[i] | large_map->map[i]);
+
+for (i = small_map->size; i < large_map->size; i++)
+or_map->map[i] = large_map->map[i];
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1, const libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+const libxl_bitmap *large_map;
+const libxl_bitmap *small_map;
+
+if (map1->size > map2->size) {
+large_map = map1;
+small_map = map2;
+} else {
+large_map = map2;
+small_map = map1;
+}
+
+
+rc = libxl_bitmap_alloc(ctx, and_map, small_map->size * 8);
+if (rc)
+goto out;
+
+/*
+ * If bitmaps aren't same size, their 'and' will be size of
+ * smaller bit map
+ */
+for (i = 0; i < and_map->size; i++)
+and_map->map[i] = (large_map->map[i] & small_map->map[i]);
+
+out:
+GC_FREE;
+return rc;
+
+}
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..a128a7c 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,13 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+/* Or and and functions for two bitmaps */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+const libxl_bitmap *map1,
+const libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ const libxl_bitmap *map1,
+ const libxl_bitmap *map2);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] new functions libxl_bitmap_{or,and}
On 4/14/2015 3:19 AM, Wei Liu wrote:
Thanks, we're getting there. If my comments confuse you please just ask
for clarification.
There is no need to change the subject line. However, it would be
useful if you have some kind of version number in you subject line. That
is
[PATCH vX] libxl: provide libxl_bimap_{and,or}
You can do this by supplying --subject-prefix= to git format-patch
git format-patch -1 --subject-prefix="[PATCH vX]" ...
What version do you want me to use at this point? I've sort of lost
count, since many changes have been style changes.
I am assuming you usually reserve new versions for substantive changes?
where X refers to your version number.
On Mon, Apr 13, 2015 at 01:47:18AM -0600, Linda Jacobson wrote:
provide logical and and or of two bitmaps
And the SoB line should be here.
What does SoB stand for in this context?
---
v.1 updated comments and format
v.2 rewrote bitmap functions to manipulate bytes not bits
Signed-off-by: Linda Jacobson
---
tools/libxl/libxl_utils.c | 74 +++
tools/libxl/libxl_utils.h | 5
2 files changed, 79 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..5c7178f 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,80 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+/* provide logical or and logical and of two bitmaps */
Normally we take the line before as the comment for the function that
follows it. So you don't need to mention "and" function here.
Actually I don't think you need a comment for such obvious function at
all.
I'll take it out.
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2)
Actually I think you need to constify map1 and map2. I.e.
const libxl_bitmap *map1,
const libxl_bitmap *map2)
How come? Out of curiosity.
Sorry for not having mentioned these earlier. Since you're going to
resend due to other issues so you might as well just take care of these
nits.
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+libxl_bitmap *lgmap;
+libxl_bitmap *smap;
+
Constify these as well.
And probably use better name like large_map and small_map?
OK. Happy to. Earlier someone complained about my names being too
verbose.
Linda
+if (map1->size > map2->size) {
+lgmap = map1;
+smap = map2;
+}
+else {
Coding style. Should be
} else {
+lgmap = map2;
+smap = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, or_map, lgmap->size * 8);
+if (rc)
+goto out;
+
+/*
+ * if bitmaps aren't the same size, their union (logical or) will
If
+ * be size of larger bit map. Any bit past the end of the
+ * smaller bit map, will match the larger one.
+ */
+for (i = 0; i < smap->size; i++)
+or_map->map[i] = (smap->map[i] | lgmap->map[i]);
+
+for (i = smap->size; i < lgmap->size; i++)
+or_map->map[i] = lgmap->map[i];
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2)
Constify map1 and map2.
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+libxl_bitmap *lgmap, *smap;
Constify lgmap and smap.
+
+if (map1->size > map2->size) {
+lgmap = map1;
+smap = map2;
+}
+else {
Coding style.
+lgmap = map2;
+smap = map1;
+}
+
+
+rc = libxl_bitmap_alloc(ctx, and_map, smap->size * 8);
+if (rc)
+goto out;
+
+/*
+ * if bitmaps aren't same size, their 'and' will be size of
If
+ * smaller bit map
+ */
+for (i = 0; i < and_map->size; i++)
+and_map->map[i] = (lgmap->map[i] & smap->map[i]);
+
+out:
+GC_FREE;
+return rc;
+
+}
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..1c0086b 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,11 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+/* or and and functions for two bitmaps */
Or
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *m
[Xen-devel] [PATCH] new functions libxl_bitmap_{or,and}
provide logical and and or of two bitmaps
---
v.1 updated comments and format
v.2 rewrote bitmap functions to manipulate bytes not bits
Signed-off-by: Linda Jacobson
---
tools/libxl/libxl_utils.c | 74 +++
tools/libxl/libxl_utils.h | 5
2 files changed, 79 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..5c7178f 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,80 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+/* provide logical or and logical and of two bitmaps */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+libxl_bitmap *lgmap;
+libxl_bitmap *smap;
+
+if (map1->size > map2->size) {
+lgmap = map1;
+smap = map2;
+}
+else {
+lgmap = map2;
+smap = map1;
+}
+
+rc = libxl_bitmap_alloc(ctx, or_map, lgmap->size * 8);
+if (rc)
+goto out;
+
+/*
+ * if bitmaps aren't the same size, their union (logical or) will
+ * be size of larger bit map. Any bit past the end of the
+ * smaller bit map, will match the larger one.
+ */
+for (i = 0; i < smap->size; i++)
+or_map->map[i] = (smap->map[i] | lgmap->map[i]);
+
+for (i = smap->size; i < lgmap->size; i++)
+or_map->map[i] = lgmap->map[i];
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2)
+{
+GC_INIT(ctx);
+int rc;
+uint32_t i;
+libxl_bitmap *lgmap, *smap;
+
+if (map1->size > map2->size) {
+lgmap = map1;
+smap = map2;
+}
+else {
+lgmap = map2;
+smap = map1;
+}
+
+
+rc = libxl_bitmap_alloc(ctx, and_map, smap->size * 8);
+if (rc)
+goto out;
+
+/*
+ * if bitmaps aren't same size, their 'and' will be size of
+ * smaller bit map
+ */
+for (i = 0; i < and_map->size; i++)
+and_map->map[i] = (lgmap->map[i] & smap->map[i]);
+
+out:
+GC_FREE;
+return rc;
+
+}
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..1c0086b 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -90,6 +90,11 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
+/* or and and functions for two bitmaps */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: provide libxl_bitmap_{and,or}
Hi all,
Just for grins, I created and unit tested a little code snippet for
'and' and 'or', using bytes instead of bits. It didn't take that long.
If you want me to replace the code I submitted with this new code, I'd
be happy to. Otherwise, I'll just make the style changes Julian
requested, (unless there is a disagreement on where the GC_INIT should
be), and merge the changes.
Linda
On 4/11/2015 2:31 PM, Julien Grall wrote:
Hi Linda,
Thank you for sending the new version.
It's common to CC the maintainers of the patch. Most of us have filter
in order to get directly mail they are involved too and avoid watching
every time the ML. Otherwise answer to your mail may take longer.
You can find them with the scripts scripts/get_maintainers.pl. I've
cced them.
On 10/04/2015 04:34, Linda Jacobson wrote:
Added functions to create the logical 'and' and logical 'or'
of two input bitmaps. Cleaned up spacing and comments.
Removed accidentally committed libxl_u_disk* files.
Thank you for saying what you changed in this version. While it's
useful during review process, this is not necessary to keep them in
the final commit message.
When the committers will commit your patch, git will ignore everything
after "---" and a newline.
For instance you could write
---
Changes in v3:
- Foo ...
Changes in v2:
- Added functions...
- Removed ...
Signed-off-by: Linda Jacobson
---
tools/libxl/libxl_utils.c | 55
+++
tools/libxl/libxl_utils.h | 6 ++
2 files changed, 61 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..fb8830a 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap,
int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
GC_INIT is a macro which declaring a variable. It seems common to
declare at the very beginning of the function. Can you move it there?
+/* if bitmaps aren't same size, the bitmap of their logical
+ or should be the size of the larger bit map
+*/
The coding style for multi-lines comment is:
/*
* My comment
* continue
*/
+size = max(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, or_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test(map1, bit))
+libxl_bitmap_set(or_map, bit);
+else if (libxl_bitmap_test(map2, bit))
+libxl_bitmap_set(or_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
Same remark.
+/* if bitmaps aren't same size, logical and should
+ be the size of the smaller bit map
+*/
Coding style
+size = min(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, and_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test (map1, bit) &&
+libxl_bitmap_test (map2, bit) )
+libxl_bitmap_set (and_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+ }
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 68b5580..0b6480d 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap
*bitmap);
+/* or, and and xor functions for two bitmaps
+ */
I saw you sent a separate patch for fixing the coding style. I don't
think it's necessary. Can you merge the 2 patches?
Although, the coding style for a one line comment is
/* My comment */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
memset(bitmap->map, -1, bitmap->size);
Other than few coding style issue, I think the 2 functions (or & and)
can be optimize (i.e working with byte rather than bit). Although, I'm
not a maintainers so I w
Re: [Xen-devel] [PATCH] libxl: provide libxl_bitmap_{and,or}
Hi Julien and all,
Julien, re: your comment on moving GC_INIT to the top.
In Wei's template for the functions, he wrote:
int rc;
GC_INIT...
/*your own stuff */
GC_FREE;
return rc;
I can move the other declarations below GC_INIT, but should I leave the
"int rc;" before it?
Thanks.
Linda
On 4/11/2015 2:31 PM, Julien Grall wrote:
Hi Linda,
Thank you for sending the new version.
It's common to CC the maintainers of the patch. Most of us have filter
in order to get directly mail they are involved too and avoid watching
every time the ML. Otherwise answer to your mail may take longer.
You can find them with the scripts scripts/get_maintainers.pl. I've
cced them.
On 10/04/2015 04:34, Linda Jacobson wrote:
Added functions to create the logical 'and' and logical 'or'
of two input bitmaps. Cleaned up spacing and comments.
Removed accidentally committed libxl_u_disk* files.
Thank you for saying what you changed in this version. While it's
useful during review process, this is not necessary to keep them in
the final commit message.
When the committers will commit your patch, git will ignore everything
after "---" and a newline.
For instance you could write
---
Changes in v3:
- Foo ...
Changes in v2:
- Added functions...
- Removed ...
Signed-off-by: Linda Jacobson
---
tools/libxl/libxl_utils.c | 55
+++
tools/libxl/libxl_utils.h | 6 ++
2 files changed, 61 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..fb8830a 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap,
int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
GC_INIT is a macro which declaring a variable. It seems common to
declare at the very beginning of the function. Can you move it there?
+/* if bitmaps aren't same size, the bitmap of their logical
+ or should be the size of the larger bit map
+*/
The coding style for multi-lines comment is:
/*
* My comment
* continue
*/
+size = max(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, or_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test(map1, bit))
+libxl_bitmap_set(or_map, bit);
+else if (libxl_bitmap_test(map2, bit))
+libxl_bitmap_set(or_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
Same remark.
+/* if bitmaps aren't same size, logical and should
+ be the size of the smaller bit map
+*/
Coding style
+size = min(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, and_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test (map1, bit) &&
+libxl_bitmap_test (map2, bit) )
+libxl_bitmap_set (and_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+ }
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 68b5580..0b6480d 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap
*bitmap);
+/* or, and and xor functions for two bitmaps
+ */
I saw you sent a separate patch for fixing the coding style. I don't
think it's necessary. Can you merge the 2 patches?
Although, the coding style for a one line comment is
/* My comment */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
memset(bitmap->map, -1, bitmap->size);
Other than few coding style issue, I think the 2 functions (or & and)
can be optimize (i.e working with byte rather than bit). Although, I'm
not a maintainers so I will let them decide if it's worth to do it.
Regards,
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: provide libxl_bitmap_{and,or}
PS - it's not much work to change to using bytes instead of bits. I
agree it would be better. I'll change it if the maintainers want.
Linda
On 4/11/2015 2:31 PM, Julien Grall wrote:
Hi Linda,
Thank you for sending the new version.
It's common to CC the maintainers of the patch. Most of us have filter
in order to get directly mail they are involved too and avoid watching
every time the ML. Otherwise answer to your mail may take longer.
You can find them with the scripts scripts/get_maintainers.pl. I've
cced them.
On 10/04/2015 04:34, Linda Jacobson wrote:
Added functions to create the logical 'and' and logical 'or'
of two input bitmaps. Cleaned up spacing and comments.
Removed accidentally committed libxl_u_disk* files.
Thank you for saying what you changed in this version. While it's
useful during review process, this is not necessary to keep them in
the final commit message.
When the committers will commit your patch, git will ignore everything
after "---" and a newline.
For instance you could write
---
Changes in v3:
- Foo ...
Changes in v2:
- Added functions...
- Removed ...
Signed-off-by: Linda Jacobson
---
tools/libxl/libxl_utils.c | 55
+++
tools/libxl/libxl_utils.h | 6 ++
2 files changed, 61 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..fb8830a 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap,
int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
GC_INIT is a macro which declaring a variable. It seems common to
declare at the very beginning of the function. Can you move it there?
+/* if bitmaps aren't same size, the bitmap of their logical
+ or should be the size of the larger bit map
+*/
The coding style for multi-lines comment is:
/*
* My comment
* continue
*/
+size = max(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, or_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test(map1, bit))
+libxl_bitmap_set(or_map, bit);
+else if (libxl_bitmap_test(map2, bit))
+libxl_bitmap_set(or_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
Same remark.
+/* if bitmaps aren't same size, logical and should
+ be the size of the smaller bit map
+*/
Coding style
+size = min(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, and_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test (map1, bit) &&
+libxl_bitmap_test (map2, bit) )
+libxl_bitmap_set (and_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+ }
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 68b5580..0b6480d 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap
*bitmap);
+/* or, and and xor functions for two bitmaps
+ */
I saw you sent a separate patch for fixing the coding style. I don't
think it's necessary. Can you merge the 2 patches?
Although, the coding style for a one line comment is
/* My comment */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
memset(bitmap->map, -1, bitmap->size);
Other than few coding style issue, I think the 2 functions (or & and)
can be optimize (i.e working with byte rather than bit). Although, I'm
not a maintainers so I will let them decide if it's worth to do it.
Regards,
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] update definition of libxl_bitmap_{or, and}
Hi Julien, Thanks for your feed back. Also, thanks for sending to all the maintainers. On 4/11/2015 2:32 PM, Julien Grall wrote: Hi Linda, As said on the previous patch, it's not necessary to send another patch in order to fix the previous one. Can you merge them? Of course, I can. I'll make all the changes you requested, and just send a new patch with all the updates. I assume it's a git option, that I didn't use. I'll look for it. I wasn't planning on doing it this way, but just followed the steps I'd done before. Git actually wanted to send two emails, so I chose the second. I'll look for the correct way to do this. Linda Regards, On 11/04/2015 05:25, Linda Jacobson wrote: Fixed a comment that included a reference to a function that was not included with these changes. Signed-off-by: Linda Jacobson --- tools/libxl/libxl_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h index 0b6480d..4852127 100644 --- a/tools/libxl/libxl_utils.h +++ b/tools/libxl/libxl_utils.h @@ -91,7 +91,7 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit); void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit); int libxl_bitmap_count_set(const libxl_bitmap *bitmap); char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap); -/* or, and and xor functions for two bitmaps +/* or and and functions for two bitmaps */ int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map, libxl_bitmap *map1, libxl_bitmap *map2); ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/2] update definition of libxl_bitmap_{or, and}
Fixed a comment that included a reference to a function that was not included with these changes. Signed-off-by: Linda Jacobson --- tools/libxl/libxl_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h index 0b6480d..4852127 100644 --- a/tools/libxl/libxl_utils.h +++ b/tools/libxl/libxl_utils.h @@ -91,7 +91,7 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit); void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit); int libxl_bitmap_count_set(const libxl_bitmap *bitmap); char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap); -/* or, and and xor functions for two bitmaps +/* or and and functions for two bitmaps */ int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map, libxl_bitmap *map1, libxl_bitmap *map2); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] libxl: provide libxl_bitmap_{and,or}
Added functions to create the logical 'and' and logical 'or'
of two input bitmaps. Cleaned up spacing and comments.
Removed accidentally committed libxl_u_disk* files.
Signed-off-by: Linda Jacobson
---
tools/libxl/libxl_utils.c | 55 +++
tools/libxl/libxl_utils.h | 6 ++
2 files changed, 61 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..fb8830a 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
bitmap->map[bit / 8] &= ~(1 << (bit & 7));
}
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
+/* if bitmaps aren't same size, the bitmap of their logical
+ or should be the size of the larger bit map
+*/
+size = max(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, or_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test(map1, bit))
+libxl_bitmap_set(or_map, bit);
+else if (libxl_bitmap_test(map2, bit))
+libxl_bitmap_set(or_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2)
+{
+uint32_t size;
+int rc;
+int bit;
+
+GC_INIT(ctx);
+/* if bitmaps aren't same size, logical and should
+ be the size of the smaller bit map
+*/
+size = min(map1->size, map2->size);
+rc = libxl_bitmap_alloc(ctx, and_map, size);
+if (rc)
+goto out;
+
+for (bit = 0; bit < (size * 8); bit++) {
+if (libxl_bitmap_test (map1, bit) &&
+libxl_bitmap_test (map2, bit) )
+libxl_bitmap_set (and_map, bit);
+}
+
+out:
+GC_FREE;
+return rc;
+ }
+
int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
{
int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 68b5580..0b6480d 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap);
+/* or, and and xor functions for two bitmaps
+ */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+libxl_bitmap *map1, libxl_bitmap *map2);
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+ libxl_bitmap *map1, libxl_bitmap *map2);
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
memset(bitmap->map, -1, bitmap->size);
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] tools/libxl new bitmap functions
Hey Konrad,
On 4/2/2015 1:34 PM, Konrad Rzeszutek Wilk wrote:
On Thu, Apr 02, 2015 at 11:38:16AM -0600, Linda Jacobson wrote:
From: Linda
Added bitmap functions for union intersection and difference betweenn two
bitmaps
Signed-off-by: Linda
---
tools/libxl/libxl_utils.c | 115 ++
tools/libxl/libxl_utils.h | 10
2 files changed, 125 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..c390ddc 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -699,6 +699,121 @@ int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
return nr_set_bits;
}
+int libxl_bitmap_union(libxl_ctx *ctx, libxl_bitmap *union_bitmap,
You have an extra space at the end..
+libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
And this should really start at the same line length as 'libxl_ctx'
Ditto for the rest of the functions.
+{
+int size;
+int rc;
+
+GC_INIT(ctx);
+
+// if bitmaps aren't the same size, union should be size of larger bit map
The comments are in:
/* Comment here. */
+size = (bitmap1->size > bitmap2->size) ? bitmap1->size : bitmap2->size;
There might be an 'max' macro somwhere in the code that you could use
for this?
Any guesses on where I might find the max macro. When I grep max, all I
find are definitions of constants that are the
max of something.
Thanks.
Linda
+
+libxl_bitmap_init(union_bitmap);
+rc = libxl_bitmap_alloc(ctx, union_bitmap, size);
+if (rc)
+{
+// Following the coding standards here.
+ //First goto I've written in decades.
Hehe.
No need to add the braces, you can just do:
if (rc)
goto out;
+goto out;
+}
+
+for (int bit = 0; bit < (size * 8); bit++)
Please move the 'int bit' to the start of the function.
+{
+// libxl_bitmap_test returns 0 if past end of bitmap
+// if the bit is set in either bitmap, set it in their union
I would change that comment to be:
/* NB. libxl_bitmap_test returns 0 if past the end of bitmap. */
+if (libxl_bitmap_test(bitmap1, bit))
+{
+libxl_bitmap_set(union_bitmap, bit);
+}
+else if (libxl_bitmap_test(bitmap2, bit))
+{
+libxl_bitmap_set(union_bitmap, bit);
+}
You can ditch the braces.
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_intersection (libxl_ctx *ctx, libxl_bitmap
There is space :^ - which should not be there.
+*intersection_bitmap, libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
+{
+int size;
+int rc;
+
+GC_INIT(ctx);
+
+// if bitmaps aren't same size, intersection should be size of
+// smaller bit map
I believe the comments I've above apply to the code below as well.
+size = (bitmap1->size > bitmap2->size) ? bitmap2->size : bitmap1->size;
+
+libxl_bitmap_init(intersection_bitmap);
+rc = libxl_bitmap_alloc(ctx, intersection_bitmap, size);
+if (rc)
+{
+goto out;
+}
+
+for (int bit = 0; bit < (size * 8); bit++)
+{
+// libxl_bitmap_test returns 0 if past end of bitmap
+// if the bit is set in both bitmaps, set it in their intersection
+if (libxl_bitmap_test (bitmap1, bit) &&
+libxl_bitmap_test (bitmap2, bit) )
You have an extra space at the end there. Don't think you need that
in libxl code (thought you do for libxc). Yeah, two different
styleguides!
+{
+libxl_bitmap_set (intersection_bitmap, bit);
+}
+}
+
+out:
+GC_FREE;
+return rc;
+}
+int libxl_bitmap_difference(libxl_ctx *ctx, libxl_bitmap *difference_bitmap,
+libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
+{
+int size;
+int rc;
+
+GC_INIT(ctx);
+
+// if bitmaps aren't the same size, difference should be size of larger
+size = (bitmap1->size > bitmap2->size) ? bitmap1->size : bitmap2->size;
+
+libxl_bitmap_init(difference_bitmap);
+rc = libxl_bitmap_alloc(ctx, difference_bitmap, size);
+if (rc)
+{
+goto out;
+}
+
+for (int bit = 0; bit < (size * 8); bit++)
+{
+/* libxl_bitmap_test returns 0 if past end of bitmap
+ if the bit is set in one bitmap and not the other, set it in
+their difference
+NOTE: if one bit map is larger, this will result in all bits
+being set past the size of the smaller bitmap; if this is not
+the desired behavior, please let me know
That would make this a bit strange. Perhaps demand that they
must be the same size? If they are not then just return an error?
+*/
+
+if (libxl_bitmap_test (bitmap1, bit)
You have an extra space at the end there. ^
+ && (!libxl_bitmap_test (bitmap2, bit)) )
+{
+libxl_bitmap_se
Re: [Xen-devel] [PATCH] tools/libxl new bitmap functions
Dario, I like your comments on naming. We'll see what others say. As far as difference, at least as I've implemented it, it's the exclusive or. To all, I've been thinking about Konrad's comment that we should disallow bitmaps of unequal size, in the XOR/difference function. I've implemented it, so the resulting bitmap is the size of the larger input bitmap, and the bits past the end of the smaller bitmap are all ones. I think this is inelegant. Another possibility, is, if two bitmaps are of unequal size, make the XOR bitmap be the size of the smaller bitmap. This would actually yield a reasonable result. What do others say? Thanks. Linda Jacobson On 4/3/2015 3:25 AM, Dario Faggioli wrote: On Thu, 2015-04-02 at 11:38 -0600, Linda Jacobson wrote: From: Linda Added bitmap functions for union intersection and difference betweenn two bitmaps Just wondering, about union and intersection, are them the best names for these operations, or do we want libxl_bitmap_or() and libxl_bitmap_and()? Personally, I'd go for _and() and _or(), it's more common and more consistent with what we have already in the codebase. About difference, what it the exact intended semantic of that operation? In set theory, AFAICR, A-B is the set of elements that are in A and are not in B. For a binary bitmap, I'd say it is the set of bits that are set in A and are not set in B, is it so? Whatever it is, we should write it down somewhere, e.g., in libxl_utils.h, as the function's doc comment. --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c +int libxl_bitmap_intersection (libxl_ctx *ctx, libxl_bitmap +*intersection_bitmap, libxl_bitmap *bitmap1, libxl_bitmap *bitmap2) Lots of long lines. Limit is 80 charathers (and keeping them a little less than that is even better :-)) To make that easier to happen, in this specific case (but also because it is more convenient in general and requires less typing), I'd use shorter variable names. Perhaps not as much as ba and bb for the operands and bi for the result (that would diverge from existing functions too much for my taste), but at least bitmap1, bitmap2 and intersect, instead of intersection_bitmap. Or even bitmap1 and bitmap2 for the operands, and just bitmap for the result. Of course, that does not guarantee that the fucntion signature stays on one line. If it still does not, you should break it. Regards, Dario ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] implementing a classic Xen front/back pv driver pair to provide a transport for 9P FS Protocol
Here is the log file inline: /bin/sh: 1: ./checker: Permission denied make[4]: *** [check-headers] Error 126 make[3]: *** [xen-foreign] Error 2 make[2]: *** [subdir-install-include] Error 2 make[1]: *** [subdirs-install] Error 2 make: *** [install-tools] Error 2 if anybody has any insights into this. Thanks. L - Original Message - From: Konrad Rzeszutek Wilk To: Linda Cc: Julien Grall , Wei Liu , xen-devel@lists.xen.org Sent: Thu, 02 Apr 2015 12:59:14 -0600 (MDT) Subject: Re: [Xen-devel] implementing a classic Xen front/back pv driver pair to provide a transport for 9P FS Protocol On Wed, Apr 01, 2015 at 07:36:49PM -0600, Linda wrote: > First, Julien, your suggestion worked like a charm. > So here's what's happened tonight. I tried to build the tools directory of > my git repository. Although I used the sudo command in my virtual ubuntu, I > got a permission denied error 126 on xen_foreign. One way to help with that is if you do: make 1>&2 2>log And attach the log. > > Second, I tried to follow the protocol for submitting my patches. I changed > libxl_utils.c and libxl_utils.h, in my repository, add and commit them. > Unfortunately, I didn't include my signature (next page of instructions I > was following), and couldn't figure how to get back in to add them. git commit --amend Or if you want to do more of them: git rebase -i origin/staging (and in the editor change 'pick' to 'r'). > > Finally, I tried git send-email (took a bit to find I had to install it). > Now it doesn't like the format of my send-email: Again, please copy-n-paste the command line you had. > to the devlopers list above and cc'ing Julien and Wei, followed by: > 1. following this with the files (even with --no-format-patch), error was > "no subject line" > 2. (different attempt) the repository "master" error complained about the > format patch Not sure I understand that. Is that for 'git send-email' or 'git format-patch'? > > SO if anyone is up at an ungodly hour and can explain any of these errors > to me (I'm in Colorado - so it's 7:30 here), especially with a fix, I'd be > grateful. Otherwise, Julien, Wei, I'll start at about 7am my time, maybe a > little earlier. The previous OPW had an article about using git and how to do it with kernel patches. It is exactly the same flow - except different email address. Anyhow, what I end up doing is: 1). git format-patch --subject-prefix "PATCH RFC" origin/staging.. (which generates 0001-, 0002-, etc files - for two patches). 2). git send-email --to xen-de...@xenproject.org --to someotheremail.org --compose --subject "[PATCH RFC] Patches to fix XYZ." 000*.patch And in the editor do a little writeup of what the patches have. At the end of this, I attach the output from: git diff --stat origin/staging.. git shortlog origin/staging.. which gives a nice diff output and what the patches are. And then send it off. > > Thanks. > > Linda Jacobson > > On 4/1/2015 2:57 PM, Julien Grall wrote: > > > > > >On 01/04/2015 18:46, Linda wrote: > >>I'll try it. That's the > >> > >>libvncserver-dev libsdl-dev libjpeg62-dev > >> > >>Should I keep the libsdl-dev? > >> > >> In the meantime, I'm following the git protocol for patches. I > >>successfully cloned xen.git. The next statement in the directions - I > >>can't tell if it's one statement on many lines, or many statements. It > >>starts out git branch -a > >>When I type this alone, I get "Not a git repository" When I type in the > >>many lines as a single command I get the error message: > >>origin/master no such file or directory > > > >You have to type the command "git branch -a" in the git repository (i.e > >the directory xen.git). > > > > > >> > >>This comes from the line remotes/origin/HEAD->origin > >> > >>? > > > >This is normal. The line starting by '$' is a command. Everything else is > >an example output of the execution of the command. > > > >Obviously, you have to drop the '$' when typing copying the command. > > > >To go further, '$' means a command to execute with your current user and > >'#' a command to execute with root privileges (i.e adding sudo before). > > > >This is usually a standard on Linux/BSD shell documentation. > > > >Although, there is some place within this wiki page where the command > >doesn't have '$'/'#' (see [1]). So you to judge yourself if the line looks > >like a command or not :). > > > >Regards, > > > >[1] http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Git_send-email > > > > > > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] implementing a classic Xen front/back pv driver pair to provide a transport for 9P FS Protocol
Konrad - Here's the log. Maybe someone can make sense of this. Thanks. Linda log Description: Binary data ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] implementing a classic Xen front/back pv driver pair to provide a transport for 9P FS Protocol
Thanks Konrad. I came close to doing your suggestions on git, and got it, sort of, to work. I will follow your instructions exactly for updates. There were also, some problems with the way the smtp server needed to be set up (compounded by typos of course), which are all fixed. As far as your answer to my build problems, I'm not sure what this will do: One way to help with that is if you do: make 1>&2 2>log And attach the log. I am semi-new to the linux world - is this to allow someone else to help me? What does the make command above do? Thanks, again. Linda On 4/2/2015 12:59 PM, Konrad Rzeszutek Wilk wrote: On Wed, Apr 01, 2015 at 07:36:49PM -0600, Linda wrote: First, Julien, your suggestion worked like a charm. So here's what's happened tonight. I tried to build the tools directory of my git repository. Although I used the sudo command in my virtual ubuntu, I got a permission denied error 126 on xen_foreign. One way to help with that is if you do: make 1>&2 2>log And attach the log. Second, I tried to follow the protocol for submitting my patches. I changed libxl_utils.c and libxl_utils.h, in my repository, add and commit them. Unfortunately, I didn't include my signature (next page of instructions I was following), and couldn't figure how to get back in to add them. git commit --amend Or if you want to do more of them: git rebase -i origin/staging (and in the editor change 'pick' to 'r'). Finally, I tried git send-email (took a bit to find I had to install it). Now it doesn't like the format of my send-email: Again, please copy-n-paste the command line you had. to the devlopers list above and cc'ing Julien and Wei, followed by: 1. following this with the files (even with --no-format-patch), error was "no subject line" 2. (different attempt) the repository "master" error complained about the format patch Not sure I understand that. Is that for 'git send-email' or 'git format-patch'? SO if anyone is up at an ungodly hour and can explain any of these errors to me (I'm in Colorado - so it's 7:30 here), especially with a fix, I'd be grateful. Otherwise, Julien, Wei, I'll start at about 7am my time, maybe a little earlier. The previous OPW had an article about using git and how to do it with kernel patches. It is exactly the same flow - except different email address. Anyhow, what I end up doing is: 1). git format-patch --subject-prefix "PATCH RFC" origin/staging.. (which generates 0001-, 0002-, etc files - for two patches). 2). git send-email --to xen-de...@xenproject.org --to someotheremail.org --compose --subject "[PATCH RFC] Patches to fix XYZ." 000*.patch And in the editor do a little writeup of what the patches have. At the end of this, I attach the output from: git diff --stat origin/staging.. git shortlog origin/staging.. which gives a nice diff output and what the patches are. And then send it off. Thanks. Linda Jacobson On 4/1/2015 2:57 PM, Julien Grall wrote: On 01/04/2015 18:46, Linda wrote: I'll try it. That's the libvncserver-dev libsdl-dev libjpeg62-dev Should I keep the libsdl-dev? In the meantime, I'm following the git protocol for patches. I successfully cloned xen.git. The next statement in the directions - I can't tell if it's one statement on many lines, or many statements. It starts out git branch -a When I type this alone, I get "Not a git repository" When I type in the many lines as a single command I get the error message: origin/master no such file or directory You have to type the command "git branch -a" in the git repository (i.e the directory xen.git). This comes from the line remotes/origin/HEAD->origin ? This is normal. The line starting by '$' is a command. Everything else is an example output of the execution of the command. Obviously, you have to drop the '$' when typing copying the command. To go further, '$' means a command to execute with your current user and '#' a command to execute with root privileges (i.e adding sudo before). This is usually a standard on Linux/BSD shell documentation. Although, there is some place within this wiki page where the command doesn't have '$'/'#' (see [1]). So you to judge yourself if the line looks like a command or not :). Regards, [1] http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Git_send-email ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] tools/libxl new bitmap functions
From: Linda
Added bitmap functions for union intersection and difference betweenn two
bitmaps
Signed-off-by: Linda
---
tools/libxl/libxl_utils.c | 115 ++
tools/libxl/libxl_utils.h | 10
2 files changed, 125 insertions(+)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..c390ddc 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -699,6 +699,121 @@ int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
return nr_set_bits;
}
+int libxl_bitmap_union(libxl_ctx *ctx, libxl_bitmap *union_bitmap,
+libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
+{
+int size;
+int rc;
+
+GC_INIT(ctx);
+
+// if bitmaps aren't the same size, union should be size of larger bit map
+size = (bitmap1->size > bitmap2->size) ? bitmap1->size : bitmap2->size;
+
+libxl_bitmap_init(union_bitmap);
+rc = libxl_bitmap_alloc(ctx, union_bitmap, size);
+if (rc)
+{
+// Following the coding standards here.
+ //First goto I've written in decades.
+goto out;
+}
+
+for (int bit = 0; bit < (size * 8); bit++)
+{
+// libxl_bitmap_test returns 0 if past end of bitmap
+// if the bit is set in either bitmap, set it in their union
+if (libxl_bitmap_test(bitmap1, bit))
+{
+libxl_bitmap_set(union_bitmap, bit);
+}
+else if (libxl_bitmap_test(bitmap2, bit))
+{
+libxl_bitmap_set(union_bitmap, bit);
+}
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
+int libxl_bitmap_intersection (libxl_ctx *ctx, libxl_bitmap
+*intersection_bitmap, libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
+{
+int size;
+int rc;
+
+GC_INIT(ctx);
+
+// if bitmaps aren't same size, intersection should be size of
+// smaller bit map
+size = (bitmap1->size > bitmap2->size) ? bitmap2->size : bitmap1->size;
+
+libxl_bitmap_init(intersection_bitmap);
+rc = libxl_bitmap_alloc(ctx, intersection_bitmap, size);
+if (rc)
+{
+goto out;
+}
+
+for (int bit = 0; bit < (size * 8); bit++)
+{
+// libxl_bitmap_test returns 0 if past end of bitmap
+// if the bit is set in both bitmaps, set it in their intersection
+if (libxl_bitmap_test (bitmap1, bit) &&
+libxl_bitmap_test (bitmap2, bit) )
+{
+libxl_bitmap_set (intersection_bitmap, bit);
+}
+}
+
+out:
+GC_FREE;
+return rc;
+}
+int libxl_bitmap_difference(libxl_ctx *ctx, libxl_bitmap *difference_bitmap,
+libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
+{
+int size;
+int rc;
+
+GC_INIT(ctx);
+
+// if bitmaps aren't the same size, difference should be size of larger
+size = (bitmap1->size > bitmap2->size) ? bitmap1->size : bitmap2->size;
+
+libxl_bitmap_init(difference_bitmap);
+rc = libxl_bitmap_alloc(ctx, difference_bitmap, size);
+if (rc)
+{
+goto out;
+}
+
+for (int bit = 0; bit < (size * 8); bit++)
+{
+/* libxl_bitmap_test returns 0 if past end of bitmap
+ if the bit is set in one bitmap and not the other, set it in
+their difference
+NOTE: if one bit map is larger, this will result in all bits
+being set past the size of the smaller bitmap; if this is not
+the desired behavior, please let me know
+*/
+
+if (libxl_bitmap_test (bitmap1, bit)
+ && (!libxl_bitmap_test (bitmap2, bit)) )
+{
+libxl_bitmap_set (difference_bitmap, bit);
+}
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
+
+
/* NB. caller is responsible for freeing the memory */
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap)
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..521e4bb 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -91,6 +91,16 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
+/*
+ union, intersection and difference functions for
+ two bitmaps
+*/
+int libxl_bitmap_union(libxl_ctx *ctx, libxl_bitmap *new_bitmap, libxl_bitmap
*bitmap1, libxl_bitmap *bitmap2);
+
+int libxl_bitmap_intersection(libxl_ctx *ctx, libxl_bitmap *new_bitmap,
libxl_bitmap *bitmap1, libxl_bitmap *bitmap2);
+
+int libxl_bitmap_difference(libxl_ctx *ctx, libxl_bitmap *new_bitmap,
libxl_bitmap *bitmap1, libxl_bitmap *bitmap2);
+
static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
{
memset(bitmap->map, -1, bitmap->size);
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] Xen Outreachy application
Hi Lars, I am aware of it. Thank you. I've been having trouble with builds and git send-emails. The code is written and minimally unit tested. I would love to have it reviewed. I'm waiting on answers/suggestions from Julien and Wei, who have been very responsive. They deserve gold stars for their patience with me. BTW, I hope you don't mind I changed the subject line, so the developers on the list understand the circumstances. Thanks, Linda Jacobson On 4/2/2015 7:15 AM, Lars Kurth wrote: Linda, I do want to remind you that you need to start communicating on the public list and submit some patches. Also some of us are away before the application deadline on the 7th. So things may get tight Regards Lars On 30 Mar 2015, at 20:26, Linda Jacobson wrote: Will do. Thanks for all the info. L Sent from my iPhone On Mar 30, 2015, at 12:52 PM, Wei Liu wrote: On Mon, Mar 30, 2015 at 12:29:41PM -0600, Linda wrote: Hi Wei, I have a few questions before we chat Tuesday. Per your instructions, I read the coding standards, which referred me to libxl.h for memory management. They are contradictory, in that the coding standards imply that I should be using internal libxl_*alloc functions, but the memory management section says to do a malloc. I thought someone should know. They are not contradictory. libxl_*alloc are wrappers to libc's *alloc functions. I think that section is mostly material for external callers (i.e. from the libxl user's point of view). You're going to work on the library itself so your point of view is a bit different. I assume I should look for and use the appropriate libxl_*alloc function. Yes. Also, you mentioned that all libxl functions should take a ctx (context?) as the first argument. I had looked at several libxl_bitmap* functions (init, test, set, and dispose) which don't, but looking at the alloc code you sent me, I'm not clear on the purpose of the ctx. Perhaps we can go over that in our chat. I think I said "usually". There are exceptions. :-) Yes, ctx means context. The application (libxl user) is going to initialise that context and pass that to libxl public API. Then your public API (the functions that you're going to implement are public APIs) can use that context to do various stuff. There are many things wrapped in context, but I don't think you really need to care about what they are unless you really need them. For the time being, you can use the code snippet (libxl_bitmap_alloc) I sent you as skeleton to implement your own functions. Wei. Thanks. Linda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] implementing a classic Xen front/back pv driver pair to provide a transport for 9P FS Protocol
First, Julien, your suggestion worked like a charm. So here's what's happened tonight. I tried to build the tools directory of my git repository. Although I used the sudo command in my virtual ubuntu, I got a permission denied error 126 on xen_foreign. Second, I tried to follow the protocol for submitting my patches. I changed libxl_utils.c and libxl_utils.h, in my repository, add and commit them. Unfortunately, I didn't include my signature (next page of instructions I was following), and couldn't figure how to get back in to add them. Finally, I tried git send-email (took a bit to find I had to install it). Now it doesn't like the format of my send-email: to the devlopers list above and cc'ing Julien and Wei, followed by: 1. following this with the files (even with --no-format-patch), error was "no subject line" 2. (different attempt) the repository "master" error complained about the format patch SO if anyone is up at an ungodly hour and can explain any of these errors to me (I'm in Colorado - so it's 7:30 here), especially with a fix, I'd be grateful. Otherwise, Julien, Wei, I'll start at about 7am my time, maybe a little earlier. Thanks. Linda Jacobson On 4/1/2015 2:57 PM, Julien Grall wrote: On 01/04/2015 18:46, Linda wrote: I'll try it. That's the libvncserver-dev libsdl-dev libjpeg62-dev Should I keep the libsdl-dev? In the meantime, I'm following the git protocol for patches. I successfully cloned xen.git. The next statement in the directions - I can't tell if it's one statement on many lines, or many statements. It starts out git branch -a When I type this alone, I get "Not a git repository" When I type in the many lines as a single command I get the error message: origin/master no such file or directory You have to type the command "git branch -a" in the git repository (i.e the directory xen.git). This comes from the line remotes/origin/HEAD->origin ? This is normal. The line starting by '$' is a command. Everything else is an example output of the execution of the command. Obviously, you have to drop the '$' when typing copying the command. To go further, '$' means a command to execute with your current user and '#' a command to execute with root privileges (i.e adding sudo before). This is usually a standard on Linux/BSD shell documentation. Although, there is some place within this wiki page where the command doesn't have '$'/'#' (see [1]). So you to judge yourself if the line looks like a command or not :). Regards, [1] http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Git_send-email ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] source code
Hi all,
The following functions should compute the union, intersection and
difference of two bitmaps.
Please review the code. Thank you.
Sincerely,
Linda Jacobson
int libxl_bitmap_union(libxl_ctx *ctx, libxl_bitmap *union_bitmap,
libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
{
int size;
int rc;
GC_INIT(ctx);
// if bitmaps aren't the same size, union should be size of larger bit map
size = (bitmap1->size > bitmap2->size) ? bitmap1->size : bitmap2->size;
libxl_bitmap_init(union_bitmap);
rc = libxl_bitmap_alloc(ctx, union_bitmap, size);
if (rc)
{
// I'm following the coding standards here. First goto I've
written in decades.
goto out;
}
for (int bit = 0; bit < (size * 8); bit++)
{
// libxl_bitmap_test returns 0 if past end of bitmap
// if the bit is set in either bitmap, set it in their union
if (libxl_bitmap_test(bitmap1, bit))
{
libxl_bitmap_set(union_bitmap, bit);
}
else if (libxl_bitmap_test(bitmap2, bit))
{
libxl_bitmap_set(union_bitmap, bit);
}
}
out:
GC_FREE;
return rc;
}
int libxl_bitmap_intersection (libxl_ctx *ctx, libxl_bitmap
*union_bitmap, libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
{
int size;
int rc;
GC_INIT(ctx);
// if bitmaps aren't the same size, intersection should be size of
smaller bit map
size = (bitmap1->size > bitmap2->size) ? bitmap2->size : bitmap1->size;
libxl_bitmap_init(union_bitmap);
rc = libxl_bitmap_alloc(ctx, union_bitmap, size);
if (rc)
{
goto out;
}
for (int bit = 0; bit < (size * 8); bit++)
{
// libxl_bitmap_test returns 0 if past end of bitmap
// if the bit is set in both bitmaps, set it in their intersection
if (libxl_bitmap_test (bitmap1, bit) && libxl_bitmap_test
(bitmap2, bit) )
{
libxl_bitmap_set (intersection_bitmap, bit);
}
}
out:
GC_FREE;
return rc;
}
int libxl_bitmap_difference(libxl_ctx *ctx, libxl_bitmap *union_bitmap,
libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
{
int size;
int rc;
GC_INIT(ctx);
// if bitmaps aren't the same size, difference should be size of larger
bit map
size = (bitmap1->size > bitmap2->size) ? bitmap1->size : bitmap2->size;
libxl_bitmap_init(union_bitmap);
rc = libxl_bitmap_alloc(ctx, union_bitmap, size);
if (rc)
{
goto out;
}
for (int bit = 0; bit < (size * 8); bit++)
{
/* libxl_bitmap_test returns 0 if past end of bitmap
if the bit is set in one bitmap and not the other, set it in
their difference
NOTE: if one bit map is larger, this will result in all bits
being set past the size of the smaller bitmap; if this is not
the desired behavior, please let me know
*/
if (libxl_bitmap_test (bitmap1, bit) && (!libxl_bitmap_test
(bitmap2, bit)) )
{
libxl_bitmap_set (difference_bitmap, bit);
}
}
out:
GC_FREE;
return rc;
}
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] coding standards
Will do. Thanks for all the info. L Sent from my iPhone > On Mar 30, 2015, at 12:52 PM, Wei Liu wrote: > >> On Mon, Mar 30, 2015 at 12:29:41PM -0600, Linda wrote: >> Hi Wei, >>I have a few questions before we chat Tuesday. Per your instructions, I >> read the coding standards, which referred me to libxl.h for memory >> management. They are contradictory, in that the coding standards imply that >> I should be using internal libxl_*alloc functions, but the memory management >> section says to do a malloc. I thought someone should know. > > They are not contradictory. libxl_*alloc are wrappers to libc's *alloc > functions. > > I think that section is mostly material for external callers (i.e. from > the libxl user's point of view). You're going to work on the library > itself so your point of view is a bit different. > >>I assume I should look for and use the appropriate libxl_*alloc >> function. > > Yes. > >>Also, you mentioned that all libxl functions should take a ctx >> (context?) as the first argument. I had looked at several libxl_bitmap* >> functions (init, test, set, and dispose) which don't, but looking at the >> alloc code you sent me, I'm not clear on the purpose of the ctx. Perhaps we >> can go over that in our chat. > > I think I said "usually". There are exceptions. :-) > > Yes, ctx means context. The application (libxl user) is going to > initialise that context and pass that to libxl public API. Then your > public API (the functions that you're going to implement are public > APIs) can use that context to do various stuff. > > There are many things wrapped in context, but I don't think you really > need to care about what they are unless you really need them. > > For the time being, you can use the code snippet (libxl_bitmap_alloc) I > sent you as skeleton to implement your own functions. > > Wei. > >> Thanks. >> >> Linda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] coding standards
Hi Wei, I have a few questions before we chat Tuesday. Per your instructions, I read the coding standards, which referred me to libxl.h for memory management. They are contradictory, in that the coding standards imply that I should be using internal libxl_*alloc functions, but the memory management section says to do a malloc. I thought someone should know. I assume I should look for and use the appropriate libxl_*alloc function. Also, you mentioned that all libxl functions should take a ctx (context?) as the first argument. I had looked at several libxl_bitmap* functions (init, test, set, and dispose) which don't, but looking at the alloc code you sent me, I'm not clear on the purpose of the ctx. Perhaps we can go over that in our chat. Thanks. Linda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
