Tracker item #2064471, was opened at 2008-08-21 18:41 Message generated for change (Comment added) made by ven9 You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=989708&aid=2064471&group_id=204462
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: toolbox Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: ven (ven9) Assigned to: Nobody/Anonymous (nobody) Summary: FEATURE:control vm recording by toolbox Initial Comment: A patch that affects both vmware-toolbox and vmware-toolbox-cmd, adding start/stop recording functionalities for them. It adds a new GuestApp routine, through vmware backdoor, to do it, a new panel for toolbox and a new command for toolbox-cmd as UI. This patch is based on 2008.08.08 release. I have already posted a version in devel mailing list, now I submit to tracker for simpler tracking. Thanks to Adar's review. More comments are welcomed. ---------------------------------------------------------------------- >Comment By: ven (ven9) Date: 2008-09-24 17:18 Message: Thanks for another reviewer. The parameter name suggestion in GuestApp_ControlRecord is good for me. Root protection for record may be not necessary from my point of view. Do we all agree with that? I checked the mnemonics in toolbox. There is a conflict between r of record tab and start button, which are all added by me. Now I change to s of start and no conflict is seen. Here is another patch for above changes. File Added: 0001-Fix-according-to-Marcelo-s-comments.patch ---------------------------------------------------------------------- Comment By: Adar Dembo (adembo) Date: 2008-09-23 09:39 Message: Yiwen, here are some comments from Marcelo. He's having trouble using his SF account so I'm posting this on his behalf; I think he's subscribed to bug updates so feel free to respond. Looks OK to me; I'd just choose a better name for the argument to GuestApp_ControlRecord; what you'd passing is not really a "mode", but a command (i.e., "start recoding" and other commands defined in statelogger_backdoor_def.h). Also, the "root protection" in the Toolbox, for this particular case, is pretty useless, for reasons I guess we all know. I'd also double-check if the mnemonics don't conflict with any existing controls in the UI. ---------------------------------------------------------------------- Comment By: ven (ven9) Date: 2008-09-18 10:35 Message: Thank you for your kindness and patience:) I am sorry for all the inconvenience. Since I started the patch before 2008.09.03, and I am new to git, I don't know an easy way to move previous work along with its history. Hopefully there are no greate conflicts. ---------------------------------------------------------------------- Comment By: Adar Dembo (adembo) Date: 2008-09-18 10:29 Message: Don't worry, it was a learning exercise for me too; I learned about git-merge and what a wonderful command it is. Since you're working within a git repo, all you should need to do is 'git pull', which will fetch the latest contents of the main open-vm-tools repo (using git-fetch) and then merge them into your repo, accounting for any local changes you've made that the main repo didn't have (using git-merge). The merge will be automatic unless there are conflicts, at which point you'll be prompted to make a three-way merge. In this case, here's what I had to do: - create a new branch named 'yiwen-patches' based on before 2008.09.03. - switch to 'yiwen-patches'. - apply your patchset (which commits them to that branch using the commit details you provided in the patchset). - switch back to the main branch. - use git-merge to merge 'yiwen-patches' to the main branch. This brought in all changes on 'yiwen-patches' which was just your patchset. There were no conflicts, so it was all automatic. ---------------------------------------------------------------------- Comment By: ven (ven9) Date: 2008-09-18 10:24 Message: Thank you for your kindness and patience:) I am sorry for all the inconvenience. Since I started the patch before 2008.09.03, and I am new to git, I don't know an easy way to move previous work along with its history. Hopefully there are no greate conflicts. ---------------------------------------------------------------------- Comment By: Adar Dembo (adembo) Date: 2008-09-18 10:11 Message: Yiwen, thanks for being so patient. I tried the latest patchset and was able to get it to apply with a little bit of branching. In the future, please make sure that the patches you send are based on the latest HEAD in the git repo (yours were based on an older HEAD, before the 2008.09.03 refresh, which necessitated some merging on my part). I'll forgo actually pushing your patches to the open-vm-tools repo until we can get them reviewed by another reviewer; I'll try and round up another member of our team. ---------------------------------------------------------------------- Comment By: ven (ven9) Date: 2008-09-17 15:16 Message: I did another experiment. The failure of applying my patches should result from that I created git control data at a cpdeeper level than the public repository did. Sorry for the mistake. I add prefix for file names in the new patches. I hope all issues addressed have been resolved now. File Added: newpatch.tar.gz ---------------------------------------------------------------------- Comment By: ven (ven9) Date: 2008-09-17 10:13 Message: Thanks, Adar. Sorry for the late response due to a local festival. I tested the patch files as follows: 1. Get an open-vm-tools tarball and extract. The source directory looks like open-vm-tools-yyyy.mm.dd-build/; 2. Enter the directory, initialize a repository (git init, git add ., git commit); 3. In the top level of source directory mentioned above, git am each patch file, or use wildcard. Since I stayed the top level of source directory, the file names in the patch look fine to me. Perhaps your git profiles are in a higher level. If it doesn't work, the file names may have to been modified explicitily. I will fix other issues and submit them soon. ---------------------------------------------------------------------- Comment By: Adar Dembo (adembo) Date: 2008-09-14 06:11 Message: Thanks, Yiwhen. The screenshots look good; no problems there. I'm still having some issues applying your patches, though: - For each patch, make sure your e-mail address is listed. Some of them show [EMAIL PROTECTED] - For each patch, provide an explicit commit message title and details. Some of the patches show "put your commit message title" as the message title and "put your commit message details" in the details. - Don't worry about "testing done" and "reviewers" if there aren't any for a particular patch. - Most importantly, all the file paths in your patches lack an "open-vm-tools/" prefix. This means that when I try to apply them using git-am, git can't find the files to patch. Could you either rebuild your patches with the prefix in place in the file paths, or explain to me how I should use git-am (or git-apply) so that I can connect the files in your patches to my files? ---------------------------------------------------------------------- Comment By: ven (ven9) Date: 2008-09-11 20:39 Message: Thank you for the comments. They sound reasonable. I have made adjust. I have made ( and tested ) patch chain along with required screenshots. They are packed in the attachment. The button layout may not be elegant for various window size, though. File Added: commit.tar.gz ---------------------------------------------------------------------- Comment By: Adar Dembo (adembo) Date: 2008-09-11 16:22 Message: I've looked at your second patch, and here is another round of comments: - I'm trying to use git's workflow to incorporate your patches. To that end, could you use git-format-patch to format your patches? That way I should be able to use git-am to apply them easily (so make sure you can use git-am locally to apply them). - I'd like to see a screenshot of the new tab you've added to the gtk toolbox. I'd also appreciate a screenshot of the error presented by Toolsmain_MsgBox. - In GuestApp_ControlRecord, refer to statelogger_backdoor_def.h without the path. That way, we could move statelogger_backdoor_def.h to a new location in the future without updating this comment. - Don't bother calling out the isolation flag in RECORD_VMX_ERR. We cover them exhaustively in the product manuals, and mentioning it by name in this code will make it harder to change the name of the flag in the future. - I have some cosmetic suggestions regarding the wording in RECORD_VMX_ERR. Here's an alternative wording: "Error, the Record/Replay control operation failed. This could be for one of the following reasons:\n" \ "1. You are running an old version of a VMware product." \ "2. Your product has disabled these controls. To enable them, consult the product documentation.\n\n" \ "3. You tried to start a recording while already recording.\n\n" \ "4. You tried to stop a recording while already not recording.\n\n" ---------------------------------------------------------------------- Comment By: ven (ven9) Date: 2008-09-10 14:04 Message: Correct my typos and incorporate the new macros. Also add some informational error message. The new patch is applied after the first one. File Added: record-control2.patch ---------------------------------------------------------------------- Comment By: Adar Dembo (adembo) Date: 2008-09-03 15:02 Message: Logged In: YES user_id=1867590 Originator: NO Looks good, just two more requests out of me: - "proccess" --> "process" in GuestApp_ControlRecord. - I'm attaching statelogger_backdoor_def.h which contains a very rudimentary record of this backdoor protocol. Could you incorporate it into your patch, using its macros where appropriate? File Added: statelogger_backdoor_def.h ---------------------------------------------------------------------- Comment By: ven (ven9) Date: 2008-09-01 17:58 Message: Logged In: YES user_id=2082796 Originator: YES Any comments? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=989708&aid=2064471&group_id=204462 ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ open-vm-tools-devel mailing list open-vm-tools-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open-vm-tools-devel
